When your code base grows you will end with dozens of places to change your code, all of them error prone. More things you need to keep control means more potential problems.
There are 2 alternatives to this. For languages that have macros (C#, C, C++) use the #ifdef construct, as others suggested here. For languages that don't have them (Java, Perl) use a less error prone alternative:
static boolean Test = false; // Only one place to change code
...
if(Test) doTest();
The difference in this last technique is that since you have just one place to change code there is less chance for errors and the change is easier to automate.
You should not have commented out code checked in. Period. Whether you are doing if(0), #ifdef 0, #if 0 it doesn't matter. It makes the code harder to read and debug (suddenly whoever comes after you can't read the whole function / context on the screen at once).
If you DO want to do this either:
a) delete the code. It'll be in version control anyway right? You do do version control right?
b) put it under a NAMED feature flag. Test is not a suitable name. Put it in a separate file which is conditionally compiled into the build with #ifdef SHINY_FEATURE shiny_feature(); #endif
Almost all of the time I agree. But sometimes code is the clearest and most understandable way to describe an algorithm, and sometimes you want to describe an algorithm for a reason other than that you want it to execute. For example, you might want to say "This would be simpler but it fails when x is denormalized," or you might want to say "This is how you could call this method," which is common in Python and elisp docstrings — and, I think, wholly defensible in that case.
Most days I don't write comments. Maybe once every 3 months. When I feel like someone might need a comment I rewrite the code until I feel like they don't need a comment.
The only time I write comments is when:
1. Someone fucked up big and shipped an iPad app or something that relies on an error in the API or something. Can't force users to upgrade iPad apps, especially on older ones who are fearful of the app breaking or getting worse, so you end up with these massive comments fully explaining the problem, the hack that solves it, the conditions that need to be met in order for the hack to be solved (for example, "check google analytics to see if the number of people using version X of the iPad app or lower is less than 1%, if it is, replace the bellow hack with the following solution"), and possibly other hacks investigated and the reasons that they were declined. The reason you do this in the code rather than just in the commit message is that it protects the message from getting watered down as people change the code. I assume that someone is in a massive hurry when they are reading code and massive wtfs should be explained without further investigation.
2. When the performance of the obvious solution is terrible and the efficient solution is complex. Especially when math is involved (for example, the core match method of a distributed recommendation engine).
> Commented-out code is not executable code, so it has no useful effect for either reader or runtime. It also becomes stale very quickly. Version-related comments and commented-out code try to address questions of versioning and history. These questions have already been answered (far more effectively) by version control tools.
Run that with vanilla JavaScript, Java, SQL or anything but C/C++ [this is the premise of the protip] and I'll bake you a cake! (One that's not a lie;) Nothing that's discussed here does the same thing, though I completely agree there is a lot of better techniques for various tasks, nothing is functionally the same as the trick. (Why can I not reply the the reply of this post? I'm new here.)
Well, first, there is nothing preventing you from running JS or Java through the C preprocessor (and we did run JavaScript through it at my last job).
But there are equivalents for any programming language, but others here have already touched on them.
I'm not sure what SQL has built in, but I'd just use the C preprocessor if I needed that. However, I'd much rather not muck with my queries like that, because that will cause nasty surprises in production.
And I don't eat carbs, so your off the hook for a cake.
> Well, first, there is nothing preventing you from running JS or Java through the C preprocessor (and we did run JavaScript through it at my last job).
This is an interesting idea! What was the problem you were solving that required this?
---
I think actually the C preprocessor would solve a lot of the problems that, say, SCSS solves too:
// put these in a header file, 'cssutil.h'
#define HASH #
#define f(x) x
#define COLOR(c) f(HASH)c
#define PREFIXED(x, y) \
-webkit- ## x : y;\
-moz- ## x : y;\
-o- ## x : y;\
x : y
#define SHADOWED PREFIXED(box-shadow, 10px)
// then in your css
#include "cssutil.h"
#define THEME_BG COLOR(fff)
div.main {
background: THEME_BG;
SHADOWED;
}
There we go, pristine(ish) CSS, unsullied by magic constants and messy prefixes! (note that my preprocessor-fu is weak, I don't know how to do variadic prefixes)
Often times having a one-size-fits-all approach (especially across languages [!]) is not a good idea.
Different languages have different optimization techniques for removing code during compilation. Java, for example [1], can optimize out based on static boolean variables. I recommend learning more about each individual language and what it offers to get the best out of it.
More sins have been committed in the cause of "Hey, this will save a few keystrokes" than . . . well, a lot of sins, okay?
I have no trouble doing this in the heat of bring-up or a debugging session, but I'd never check it in, and I hope that none of my cow-orkers ever will, either.
But he's right. It's a slippery slope to bad things when keystroke convenience takes precedence over clarity. We're human after all; it's easy to overlook something critical (especially when there's not another set of eyes going over it) and spend a lot more time and energy hunting for it later.
The disadvantage of using pre-compiler commands is that some IDEs (Xcode specifically, perhaps they all do this) don't color disabled code any differently, so at a glance you can't tell that the code block won't run.
Before a commit I'll sweep through and delete the intermediate code blocks that I've commented out. If I use pre-compiler flags to comment out the code, it's easy to miss it in the sweep. Because of this I never use the pre-compiler flags for something I don't want to get committed.
> When your code base grows you will end with dozens of places to change your code, all of them error prone.
Well, it's really only error prone if your editor doesn't have syntax highlighting. Seeing a whole swatch of code turn into the comment color pretty much tells you what's going on.
Still, I think personally I'd rather do an "#if 0". It's still only 1 character to toggle the code on or off...
I think the concern was more that the functions you are toggling will likely interact with other functions you are toggling. So if you toggle one on but forget to toggle the other, then you could end up with undesirable results.
I knew the original post left a bad taste in my mouth but wasn't sure of a better option. I really like this way better as it easily translates across any language.
My only issue using this with things like JS, won't all that code still be loaded into memory giving a misrepresentation of the memory profile of your app?
> My only issue using this with things like JS, won't all that code still be loaded into memory giving a misrepresentation of the memory profile of your app?
Yes it will. The alternative for that might be a modular design with very loose coupling and using Inversion of Control. Then, the only change you would need to make would be in the Inversion of Control configuration to load either the test code or the production code. You could place all the debug and trace code inside a single unit and use an empty stub in production. This would avoid sending the test code to the client on the production environment.
I am not an expert in JS, please correct me if this is wrong. I'd do something like this (for checking pre-conditions in method calls):
function Assert(condition, message ){} // this module is loaded/included in production only
function Assert(condition, message) { if(!condition) alert(message);} // this module is loaded/included in tests only
If you are sprinkling "if (test)..." throughout your code, you are doing it wrong.
Better would be to collect that code into one or two files at can be included when needed. I would imagine that's really only a concern when downloading code because the compiler should throw dead code ("if false...") away.
"This trick below can be used to toggle between two sets of code, with one symbol in the code with no additional short-keys to learn, and no tool dependencies!"
So can #ifdef TEST / #ifndef TEST , which also has the advantage of not being horrific.
I'm glad to know I'm not the only one abusing compiler macros for "ghetto source control".
Having said that, there are plenty of cases where you can't use macros (like all languages/compilers/interpreters that don't support them). I don't agree with the author here though. Just comment the damn code out normally until you can clean everything up. Doing it that way encourages leaving that mess intact and forgetting about it.
You and I may know this should never be committed. Hundreds of students and junior developers will read this post and
need to be warned that this is a VERY BAD IDEA.
I would rather make angry comments here than to have to explain why this is a terrible idea in a code review of an intern or a junior colleague.
Personally, I think the comments highlight why this can cause problems and what the alternatives are. Knowing the trade-offs and then choosing an approach is good design. Feel free to use that hack, but be sure you know what the trade-offs are, and what bugs can potentially arise in the future due to this hack.
Yikes. At least give the next coder that will maintain your code (and perhaps your future-self) a chance at understanding it. If the goal is to switch between prod and test code by flipping one char, then this works, too
Yes, and this way the inactive code is still checked for validity and does not leave -Wunused-* warnings. Use `#if 0` if you want to disable code that is intentionally invalid, then grep your codebase for `#if 0` and delete those dead code blocks with extreme prejudice.
I'm going slightly off-topic here, but I have to rant a bit on code reviews.
For the love of god, please, they are meant for checking each others code for mistakes and not for miniscule details like this.
I have seen it time and time again in various companies where code review is used as some sort of tool to dictate style and preferences to each other. Wasting valuable development time and creating a unhealthy tension between developers.
If code reviewers were just for checking mistakes, then unit tests would be enough.
Code reviews are invaluable in education other developers about your code, how to support it and how to develop on it. But information should also flow the other way and the give the reviewers the opportunity to present the accepted practices to the developer. In the long term, this saves developer time as the code base stays coherent. Code is read many times and needs to be understood by many developers even though it is written only once.
I think 76 characters is a good ideal to strive for because it means the code is legible everywhere, including my phone. It also reduces variables named request_response_header_validating_auditor.
However, some things don't break well, so readability ought to trump.
While I agree with you in general, in this particular case, we can run into problems when compiling the code on different platforms. Many teams/companies probably don't have a need for that. But I have been on cross platform projects where such errors would lead to days of wasted debugging time.
If you are compiling on multiple platforms, then stuff like this is a disaster.
If you are not compiling on multiple platforms, then complaining about stuff like this because of compiler issues is a nuisance. (You could still object on the basis that it is fugly.)
Spraying horrible garbage cleverness in code, and submitting commented out code, are exactly the sort of newbie mistakes that code reviews should sort out.
At best, this gimmick is for hacking out some temp code while debugging, and should not be submitted.
Of course, one should just use an IDE, or even vim, to commenting out code as needed.
/*/ Version one, currently commented
/*/ Version two, currently active /**/
And a single star switches from version two to version one.
But I agree this shouldn't be left in production code; it's just a handy trick for switching between two blocks of code†. An #ifdef is also good, but that's C/C++ only.
† Whoever suggested that some editors can comment a block of code missed the point utterly - which is to switch between code blocks easily.
The trouble with this is that you now have to switch "commented" to "active" and vice versa, when I think the aim was to have a single-char switch (although the syntax is pretty horrible, to be honest).
It's also unreadable unless you know the trick or see it, making future maintenance by others difficult. Clear and simple is usually better than complicated and tricky.
I think this is better because it's much easier to see what you're supposed to do in order to switch the block on or off, and it's only a single character change per block. (though you can't switch from test to production in one character, but meh).
However, I tend to see commented code as ugly cruft. To me, comments are for explaining in natural language what you're doing, not for deactivating bits of code. If it's bad get rid of it, if it's good keep it. Why comment code?
The only time I comment out code is to warn developers away from doing something which is obvious but a bad idea, for example:
/*
// This was a neat idea but it makes it break if the user isn't logged in
userPreference = getPrefFromUserSession();
*/
I think a better way of doing that is to create a test that fails in exactly that condition.
You can then put documentation in that test, as that's what the developer will look at first. You also get the bonus that, if someone finds a way to fix the function that causes that, then you can go back to possibly nicer code.
Reading tests is not TDD, it is basic due diligene when learning code. Eve still, skipping that, when the test fails, the rush coder will see the problem.
No, no, no, no, no! This is a terrible idea, and will end up causing you a massive headache in the long-run. If I see commented-out code like this, I will delete it because that's what version control is for.
If you really need to turn code on/off in different environments, you should be using feature flags: http://code.flickr.net/2009/12/02/flipping-out/ and at the very least you should be detecting the environment:
if (ENV == 'dev')
testStuff();
else
productionStuff();
This is much more meaningful to other developers (and to yourself in the future).
This will compile to different things depending on whether you are using a C or a C++ compiler. Worse, it may compile to different things depending on which standard you compile to. If you don't want you colleagues to hate you, you should probably use:
I don't understand why someone would do this nested comments thing. This is what macro block are for...
And tests need to be separated from the code. Hopefully using a test framework.
If your code changes depending on if it is in 'test mode' or not (which is what is proposed), then the test code is not the same as the production code, i.e. the production code is not tested. I'm not just talking about timing issues - extra variable names and objects will be in scope in 'test mode', and might logically change the behavior of the program.
We don't do that here. However, we do have an unparalleled menu of pedantic critique, edge case obsession, and a wonderful sauce made from insanely perfect standards.
Better yet, use a code review system, then automate a test that detects this and rejects outright any patch containing it. Sure, you can use this in your private repo, even keep it in your stashes/branches, but if it gets pushed to the central master, expect to be slapped down, hard.
I use this "trick" a lot, but I'll never ever dare committing it; just for small and quick checks (of the "what if I use this other implementation for the loop?"). If I'm going to commit, if (1) or #if (1) (plus a small comment explaining why I resort to that) are better options and immediately understandable for any other team member.
As a long-time C++ developer I wish people don't use any commenting tricks such as this, and I wish people don't use /* * / to comment their code, even if you develop in C. Instead, use // in modern C/C++.
The reason is that we often need to comment OUT code during development. In C++, the 4 choices are: (1) #ifdef, (2) bool dothis = false; if (dothis) { ... }, (3) /* * /, (4) // ...
Among them, I would say the best and least-confusing way to comment out a block of code is /* * /. #ifdef's are very confusing especially if you have multiple of them; // needs to be applied to every line so only works in small scale; if (dothis) doesn't work across multiple functions, and at a glance it is harder to discern whether this is a "comment-out", or a legit condition.
However, by using /* * / in your comments, you eliminate the possibility of commenting out blocks of code that contains /* * / comments. The problem, of course, is due to how the opening and closing symbols are matched.
"I'd appreciate if you'd have this discussion on Coderwall. That way I could 'defend' the post where it's due.
However though, please read https://news.ycombinator.com/item?id=5626456 and summary is:
It is a trick, not a silver bullet. It prevents code from being compiled to the binary, in a portable - yet hacky - way."
Ha, I thought I put in the asterix-esses. You know what I mean, but I bet cpp actually ignores e everything after #if 0 and #endif, don't do that please ;)
I understand the criticism, it's not supposed to be _the_ way to toggle code in and out.
It's a TRICK, and that's all it will ever be.
However, it is the only way I know of that is not language specific for a particular dialect of C (#ifdef) that enables code to _not_ become a part of the compiled binary, or in JS terms - not available from the runtime.
This is a nice trick for quick comparison and thats it.
I actually use this a lot for performance comparison.
This way it is easy to toggle between the olt d and the new implementation and see how long it takes to run the code 1000 time in javascript.
But I think anyone who check ins something likes this should be slaped.
Slightly relevant.. in an introductory C ourse at my university, we had to write a program that would tell us which version of the standard was used by the compiler. I can't think of a solution now but I'm sure it was based on different comment syntax.
I've been using this for a while in my JS projects. I'm about to start my first job as a web developer for a medium-sized company so I'm glad I came here and read all the backlash to this technique!
It's a fairly subtle way to dramatically change the behavior of your code. Frankly, it's a timebomb threatening to run testCode() in your production site.
The low cost makes it useful for quick development, sure, but for stability's and sanity's sake, please remove when you start promoting this above your own playground.
I think the poster might be referring to using finicky, unreadable line noise. Expressing program logic this was will inevitably lead to misunderstandings.
Why would you do this instead of #if 0/#if 1? Has exactly the same effect, is a one character toggle (sure, it's editing one character rather than inserting/removing one character), and is much easier to read and remember.
This trick only works if there isn't a * / inside a string in commented out code (or there isn't a * / within the commented-out code itself in a language that allows such syntax, e.g. GrOOvy).
A much more elegant solution is to use Commentary[1] in Vim (and I'm sure there's an emacs equivalent too). Just use visual mode to select your text, hit `gc`, and voilà.
During debugging, there's often the need to temporarily disable a few lines of code to see the effect it has. I'm certainly not going back-and-forth with source control for something so trivial.
That said, macros do make sense in this case- you can #define a single flag (e.g. FLAG) as a 0 or 1 and simply #if FLAG ... #endif, but it's certainly more typing to set up.
When your code base grows you will end with dozens of places to change your code, all of them error prone. More things you need to keep control means more potential problems.
There are 2 alternatives to this. For languages that have macros (C#, C, C++) use the #ifdef construct, as others suggested here. For languages that don't have them (Java, Perl) use a less error prone alternative:
static boolean Test = false; // Only one place to change code
...
if(Test) doTest();
The difference in this last technique is that since you have just one place to change code there is less chance for errors and the change is easier to automate.