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.
I think the poster might be referring to using finicky, unreadable line noise. Expressing program logic this was will inevitably lead to misunderstandings.
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'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.
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 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.
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.
"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.
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 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.
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!
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 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).
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.
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.
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.
129 comments
[ 2.0 ms ] story [ 48.6 ms ] threadAdditionally, source control tools make deleting code much more convenient, no?
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.
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.
Compilers are not uniform in their interpretation of nested comments.
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.
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.
We use various automated techniques to pick up mistakes and use code review to ensure style is consistent.
Everyone in the company should be able to parse any piece of code without individual styles disrupting that.
The only tension occurs around the weighty question of line length - coding standard says 76 chars max, I say that's what the IDE's for.
However, some things don't break well, so readability ought to trump.
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.)
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.
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.
And every language has the IF construct.
And of you are flipping between two implementation so frequently, your actual problem is that you have no idea how your program works.
So can #ifdef TEST / #ifndef TEST , which also has the advantage of not being horrific.
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.
I mean, if you're talking about basic C, this (_DEBUG) is actually built-in to most systems from the get-go, for exactly this purpose. "Abuse?"
https://github.com/scrooloose/nerdcommenter
Or switch to block selection mode, make a multi-row zero-column selection and type //
I love Eclipse.
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:
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.
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:
This is much more meaningful to other developers (and to yourself in the future).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.
Please read the protip, understand my intention (keyword TRICK) and have a delightful cup of coffee. You've earned it.
> understand my intention
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.
At least in my IDE anyway.