Comenting out code

Today I’m going to rant about something that really bugs me – commenting out code. Just about everywhere I have worked and in most code I have read I have found code that has been commented out. What is it that possesses people to do this? Why is it so prevalent? Is this practice actively being taught and recommended?

If code should be written as if for humans to read and incidentally for computers to translate then commented out code is worse than useless because it slows down and confuses the human reader.

Imagine reading a book or article with text crossed out like this. It would leave you wondering if you should be reading these sections or not. Why didn’t the author just get rid of the text all together?

When I see commented out code I don’t know if the person who did it was just forgetful, uncertain of what he or she was doing or doesn’t use or trust source control to do its job.

During development it can be useful to temporarily comment out some code to see the effect removing or replacing it will have but this temporary experiment should never be checked in.

Here are some things I do to make sure I don’t forget to remove or uncomment code in this situation:

• For real quick tests I’ll delete or cut the code and let the editor undo or paste buffer remember it for me. This has the risk of losing the old code if the editor or computer crashes but you can always get it back from source control if it was previously checked in.
• I always add a marker to the comment such as “todo” or three x’s so that I can easily find things I forgot to cleanup.
• Before checking in I diff all files against the head revision looking for the marker or other undesired or accidental changes

If you’re not sure of your changes keep reading and testing until you are sure. Then delete that old code. If it helps imagine that the code is cheese and wrapping it in comments is like leaving it out of the fridge. Its going to get smelly and you don’t want your code to stink do you?

You really don’t need the old code stuck in comments. Source control does a much better job of showing you what the code used to look like. It is more trustworthy and can tell you more information such as who made the change and when. Depending on the source control system the changes can be correlated with related changes over many files.

Are there any exceptions – any legitimate reasons for having code in comments? Just a few but most of what I come across doesn’t fall into these categories.

Code that enables a special debugging, testing or profiling mode can be left in to make it easy to re-enable that mode later. The code should have a comment such as “uncomment the following line to enable debugging”. Often there are better ways to do this. It may be useful to have a runtime switch to enable the code or if your language has ifdefs they can be used to create a variant of your program. AOP or code generation may also be appropriate.

Sometimes you may add some code that depends on some other code that isn’t ready yet. You can test your code with a test harness but it can’t be used until the other code is available. In this case there should be a comment such as “uncomment this block once the foo modules is ready”. Again there are sometimes better ways to handle this situation such as creating a branch in source control for the unfinished work with stubs in place.

Another possible reason for leaving in the commented code, that I don’t really agree with, is as a cautionary statement. As if to say don’t think of changing the code back to this because it used to be this way and I decided that it didn’t work out so well. In this case it is much better to replace the code with a human readable comment about what should or should not be done in this context. For example suppose you have code like this

  resourceA.lock();
  int foo = bar();
  ...

Someone finds a deadlock condition due to calling bar. So they comment out the call.

  resourceA.lock();
  // int foo = bar();
  ...

This is bad because future maintainers have no idea why that was done. The following is a little better.

  resourceA.lock();
  // this was causing a deadlock sometimes
  // int foo = bar();
  ...

But this is even better still.

  resourceA.lock();
  // be sure not to call any functions such as bar that try to lock additional resources
  ...

The bottom line is don’t check in commented out code and if you do it must have a human readable statement as to why it is being left in. All of the above apply to ifdef-ed out code as well.