Friday, October 9, 2009

Don't comment it out - Delete that code!

You have a big, ugly, huge block of code that you want to improve. The first thing you do? Comment it all out. Then, try to re-implement in a better way. Finally, you just leave the commented out code in the file in case you need to reference it in the future. I've been guilty of this a lot in the past but I've recently come to the conclusion that instead of commenting it out, you should delete it immediately. Here's why:

It's bad code

You've already identified this code as bad. You don't want it to exist. If you keep it "commented out" so that you can reference it, you're referencing bad code. Why would you want to do that? If the code were good, you wouldn't have commented it out, right? Since it's bad, why would you possibly want to use it. If you aren't going to use it, why keep it around?

Copy and pasting of commented out code

I also notice that I sometimes will copy and paste parts of that commented out code in my new implementation. My realization is that if I'm actually using the code that I commented out, I should either:
  1. Stop using it. I commented it out for a reason.
  2. Don't comment it out. If it actually is good code, even if it's a small part, why did I comment it out? I should have left it in.

Leaving it in for posterity

A lot of code that gets left in "so you can see how it used to work." If you want to see how it used to work, you have version control, right? Just look at the history of the file and diff the current file. Problem solved.

Big block comments detailing how something works or what it does

These are also bad. If you find yourself writing one, it means that this part of your code is probably doing too much; split it up into understandable chunks. If you need an explanation of what the code is accomplishing, your unit tests are not descriptive enough. The only exception is if the code implements something intrinsicly complicated such as an an RFC, where even someone with domain knowledge may have difficulty understanding it (example: in RFC 2822, WTF is an addr-spec and what characters are allowed in a domain literal? I don't remember and I implemented it). In this case, two things are useful:

  1. Reference the RFC (or other standards documentation) by section and subsection. If I'm implementing an addr-spec reader, I can write "This implements the addr-spec in RFC 2822, section 3.4.1 (page 16)" and now someone reading the code can go read the RFC if they need to know more about what the code is doing.
  2. Write a functional spec. I know, in Agile we favor working software over comprehensive documentation. That's fine. Just because we favor working software doesn't mean we never document. A functional spec of complex logic is hardly "comprehensive documentation;" it's a useful vehicle for clarifying how something works or how different components interact. If that the most effective solution, there's nothing non-Agile about doing it.

In conclusion

If you find yourself commenting out code, ask yourself if you really need to keep any of it and if so, then don't comment that part out. Delete everything else immediately. I promise you that you won't miss them.