Is commented out code really always bad? [duplicate]

Question

Practically every text on code quality I've read agrees that commented out code is a bad thing. The usual example is that someone changed a line of code and left the old line there as a comment, apparently to confuse people who read the code later on. Of course, that's a bad thing.

But I often find myself leaving commented out code in another situation: I write a computational-geometry or image processing algorithm. To understand this kind of code, and to find potential bugs in it, it's often very helpful to display intermediate results (e.g. draw a set of points to the screen or save a bitmap file). Looking at these values in the debugger usually means looking at a wall of numbers (coordinates, raw pixel values). Not very helpful. Writing a debugger visualizer every time would be overkill. I don't want to leave the visualization code in the final product (it hurts performance, and usually just confuses the end user), but I don't want to lose it, either. In C++, I can use #ifdef to conditionally compile that code, but I don't see much difference between this:

/* // Debug Visualization: draw set of found interest points
for (int i=0; i<count; i++)
    DrawBox(pts[i].X, pts[i].Y, 5,5);
*/

and this:

#ifdef DEBUG_VISUALIZATION_DRAW_INTEREST_POINTS
for (int i=0; i<count; i++)
    DrawBox(pts[i].X, pts[i].Y, 5,5);
#endif

So, most of the time, I just leave the visualization code commented out, with a comment saying what is being visualized. When I read the code a year later, I'm usually happy I can just uncomment the visualization code and literally "see what's going on".

Should I feel bad about that? Why? Is there a superior solution?

Update: S. Lott asks in a comment

Are you somehow "over-generalizing" all commented code to include debugging as well as senseless, obsolete code? Why are you making that overly-generalized conclusion?

I recently read Robert Martins "Clean Code", which says:

Few practices are as odious as commenting-out code. Don't do this!.

I've looked at the paragraph in the book again (p. 68), there's no qualification, no distinction made between different reasons for commenting out code. So I wondered if this rule is over-generalizing (or if I misunderstood the book) or if what I do is bad practice, for some reason I didn't know.

Answer 1

The benefit of #ifdef's as opposed to commenting it out, is that (on large projects) you can have the defines listed in a make or config file - and so don't have to manually go an uncomment things, build, and then re-comment them if it's in many places. The downside to this is that changing the project's DEFINE's will usually mean rebuiling the whole thing, not just changed files.

Though... I think the "commented out code is a bad thing" really refers to dead code that people just didn't want to delete for whatever reason (fear of throwing away something they've spent time on perhaps?). It's not really about the situation you have going for you.

Answer 2

If it's commented out, then it may rot: when you suddenly decide that you need it again, you realise that it no longer compiles, or needs to be rewritten to make it fit with other things that have changed in the meantime.

If you leave the code in, but in such a way that the compiler can optimize it away if not needed, then you benefit from keeping the code up-to-date and not having to suffer the added binary size or runtime performance.

For example, in C, this is at risk of rot:

/*
doSomeDebugStuff();
*/

And so is this:

#if 0
doSomeDebugStuff();
#endif

But this is good, since it's always checked for validity by the compiler, but likely to be optimized away:

if (0)
{
  doSomeDebugStuff();
}

Edit: as others point out, using a meaningful symbol rather than 0 for the test is even better.

Answer 3

// The problem with commented out code is that it can hide what you're actually
// trying to say in a wall of text.  Syntax highlighting may help, but you're 
// still left with this huge ginormous wall of text in front of you, searching
// through it for what the actual answer is. You'd think that mentally it'd be 
// really easy to disregard the comments, but in actual usage, it's a lot harder
// than a one word answer that can tell you what you're looking for. It's tough.
/* Sometimes they'll even through you off with different comments. */ Yes.
// It's really tough to deal with people that don't like to use source control, 
// that would rather comment lines and lines of code instead of deleting the 
// code and checking in revision.  Cue irrelevant paragraph about the reason why 
// I wrote this instead of just deleting the entire block and replacing it 
// with my intention. Maybe I was hedging my bets? Cue misspelled wrod.  

Answer 4

I think a distinction needs to be made between commented out code that is obsolete, and code that is used only in a debug build (or another "special" build, conditionally compiled in). The latter is a common practice, and nothing is wrong with it.

The former should not be present in a source base, as the version control system keeps track of the obsolete stuff, should you ever want to get it back.

Answer 5

There's almost no "always" in coding :) If you know enough about the reasons behind a guideline and have really good reason for breaking it, then do so.

For instance I comment out code when I do "kamikaze-refactoring" and need a visual reminder to add things back or remember how the old code worked for a while. It's crucial in instances like this that you'll delete the comments later on though otherwise they'll simply clutter your code.

Answer 6

Sometimes you put code in your comments to show how to use a class - this is very different from comment out code that used to run.

Answer 7

I do a lot of code reviews and I find there is no real excuse for commented code regardless of reason. If you are using the commented code for debugging purposes you can create a trace mechanism that is disabled in release mode or has tracing levels (always good to be able to trace in a release version) or you can simply use a debugger.

Commented code is bad because when other people read the code - especially when you are stressed trying to fix a bug when the original author is away on vacation - is that it is very confusing to read the code, especially if the error is from a wrongly placed // at the start of the line... even using /* you may by accident comment something that should have been in there - or not.

To keep your code clean and more readable, remove commented code, its already difficult to read programs as it is without having to read code that may or may not be significant.

Answer 8

Yes, it is.

Even you have debugging code that you do not want in your production release, you should not comment it out. Using #ifdef's is better, because then you can turn debugging on and off easily with a #define macro, or by having a separate build configuration. This definitely beats having to go into the code and commenting/uncommenting every block of the debugging code manually.

And if you need to have flexibility to turn on some debugging blocks but not others, then you should group the debugging blocks into "debug levels".

A better solution would be to not use the pre-processor at all, and to use native language features, like constants and if-statements. So instead of

#define DEBUG0
#ifdef DEBUG0
  // debugging code
#endif

you can have

constexpr bool DEBUG0 = true;
if(DEBUG0)
{
  // debugging code
}

The advantage of doing this, over using the pre-processor is that your debugging code will always be checked by the compiler. This decreases the likelihood that it will rot, when you change the code around it.

You would not suffer any performance hit if you make the boolean flag false, because modern compilers optimize away unreachable code. At worst, you might get a compiler warning about it.

Answer 9

I don't think what you're doing is out and out bad but I think at the very least you should have some actual comments with it to explain what it's doing, why and how and when to use it.

Personally I'd normally put this sort of thing in some sort of IF DebugMode = TRUE type effort around the code in question and set that as a command line / start up parameter. For me that makes it easier to see that that's why the code is there and how to set it though in your instance there might be performance issues with even that small comparison that you wish to avoid.

So I'd probably see what you're doing as a necessary evil rather than out and out wrong. If you can find someway to streamline it then obviously do so but I wouldn't beat yourself up about it.

Answer 10

I think that one of the reasons that commented out code is considered a code smell is that it indicates either that the programmer who put it there doesn't understand their source control or that they are not using any. Either of which cast further doubts on a lot of the other things they are doing as well.

If you have a legitimate reason for it and you leave an explanation for why it is a legitimate reason where it can be found you're probably doing fine. Most things that are generally considered to be bad news can also be a useful tool in the right hands. The problem is that those hands are rarer than people who think they have them.

Answer 11

It helps to provide a history of how the programmers' mind(s) were working at the time. But in these days of ubiquitous source control there's no reason to leave it in a final cut - the previous versions will contain the older code should you ever need to refer to it.

Answer 12

I don't think there are absolutes here. I sometimes leave commented out code when it's just a small snippet, especially if there is a reasonable chance that I will uncomment it again soon. Basically I leave those snippets as long as they don't disrupt the readability of the actual code, and as long as they don't multiply everywhere.

What I absolutely reject is full methods of commented out code. Yes, I've seen those before: WTF. They can rest in source revision heaven ;-)

Answer 13

Use comment if your program are going to be modified by other people, or you are modifying other people's work. You don't want other people delete your code and then later on he found that your code was better. Also if you submit a program modified by you, which was wrote by other, and then the program fails, you better pray you've only commented out the original code by your colleague so that you can roll back the change. I am saying the situation when the company don't use a version control system.

You also comment your code if you would like to reference your code later, for example you are trying different approach for the same goal.

Comment does not affect performance for compiled language or virtual-machine based language.

If one found comment really annoying, he can use a better IDE with function to hide some selected comments.