Comments Considered Dangerous

Once in a while when I need a break I take a look at Facebook. This is usually a mistake as there's very little of substance going on there. In one of the programming groups I follow, someone made a post deriding programmers that don't comment their code. I made the following statement:

Every comment is an admission by the programmer that they couldn't figure out how to write the code cleanly in the first place. Make your code understandable. Reading code should be like reading a book -- you don't need the margin notes to understand a book do you?

That seemingly innocuous statement generated a lot of discussion (and flames) that completely missed the point. And yes I was a bit snarky when challenged. It's not that comments are bad per se, but they should only be used when it's not possible to get the point across any other way.

Back in the stone ages when I was programming using stone knives and bearskins, the generally accepted best practice was to be profuse with comments. Indeed, my computer science professors had marked us down for not fully commenting our pascal programs. I have raked young programmers over the coals for a lack of comments. But the thinking on this has changed substantially. Kevlin Henney has a great talk on coding style and makes a strong case for eliminating comments as much as possible. (Amusingly, he takes a shot at Oracle's "Hello, World" example, which is a full page long and includes all the legalese.) Five years ago I'd have disagreed with him. Today, I'm pretty much in agreement.

Commenting in the high level languages we have today makes less sense than profusely commenting your assembly code.

So where are some places we shouldn't comment and instead should endeavor to write clean, readable code?

• Anywhere we can replace a comment with a good variable or function name:

i++; // Increment the product counter

does not read nearly as well as:

product_counter++;

• Anywhere we can replace a block of code with a good function name:

// Swap input voltages

c = b;
b = a;
a = c;

does not read as well as:

swap_input_voltages(&low_voltage, &high_voltage);

• Anywhere we can replace several functions with a single function that brings them together into an understandable unit:

// Swap and Total Voltages

swap_input_voltages(&low_input_voltage, &high__input_voltage)
swap_output_voltages(&low__output_voltage, &high_output_voltage)
input_voltages = sum_input_voltages(low_input_voltage, high__input_voltage)
output_voltages = sum_output_voltages(low_output_voltage, high__output_voltage)
total_voltage = input_voltages + output_voltages

does not read as well as:

total_voltage = swap_and_total_voltages(low_input_voltage, high_input_voltage,
                       low_output_voltage, high__output_voltage)

If the reader needs to know what "swap_and_total_voltages" really does they can easily dive down into the function. A function should only do one thing, but sometimes that one thing is to combine several functions into something easily understandable. 

• Anywhere we can replace a comment with a test:

// This should produce a total volatage > 10, if not we have a problem
total_voltage = input_voltages + output_voltages

is not even on the same planet as:

assert( total_voltage > 10, "problem with volate max sum calculation")

A comment reminding us of things to consider when we debug is far inferior to a unit test that will fail when this problem occurs.

• Comments that are just obvious:

// Add a and b and store in c
c = a + b

Really, does that comment actually relay any information? I'm pretty sure my grandmother would have understood that code without the prompting. And yet there is a lot of this floating around. (Kevlin's talk has a hilarious example of this.)

• Comments of a historical or transactional nature:

// 5-15-12 - GDL: Removed impedance calculations, don't really need them here.
// 4-15-13 - GDL: Fixed bug 3323464

Eek. I used to do that a lot. And I often forgot to include those notes sometimes. The proper place for this type of information is in the commit for the source control check in. If you want to be able to match the bug fix with the code change, put the bug number in the commit and put the revision number in your bug tracker. Removed impedance calculations? It sure would be nice to know what they heck they were five years later when you're debugging it.

• Comments that attempt to explain away complexity:

// Take the average for the transmorgrification replication halved
t = average((a^b)/c*(sqrt(d)+e^sum(replicated_values)/2.0))

I'm not going to attempt it because that code is meaningless, but I'll but I could come up with a set of function calls that would cleanly explain what was going on there. Each of the steps would be testable as well as the behavior, so if we ever need to debug it we can get right down to it. Oh, and what is the order of precedence in that statement?

Complexity is a code smell.

• Humor.

// Abandon all hope yee who enter here. Seriously, I have no idea why this works and
// neither will you. Unless you think you're smarter than me.

Those tidbits in the code added at 3:00AM are amusing, but a lot less amusing when you're doing a code review with some suits from a Fortune 200 company that wants to make an acquisition...

There are a lot of other examples, but in general if you can make the code clear enough that it doesn't need the comments, then do that. If you can't, then by all means put in the comment. Code that isn't clear enough to easily read generally has lots of problems anyway.

Some examples of where comments are good?

• Special comment blocks, such as for Doxygen. Being able to automatically generate documentation is great, but it also adds a lot of noise to the code. This is less of a problem nowadays that most editors support code folding. 
• Documenting parameters for functions that are meant to be external interfaces. Don't use that as an excuse to have parameter names such as "a" instead of "lower_voltage_line". If you're going to document things such as acceptable ranges of input parameters then make sure your code will complain in the case of invalid entries. You also should obviously have unit tests that makes sure those invalid parameters get complained about.
• Explaining choices. If you made a design choice in the way you implemented the algorithm it's perfectly acceptable to explain that to your reader. If you've done something after a lot of thought, save your future reader (it might be you) some hassle and explain it:

// 
// Dear maintainer:
// 
// Once you are done trying to 'optimize' this routine,
// and have realized what a terrible mistake that was,
// please increment the following counter as a warning
// to the next guy:
// 
// total_hours_wasted_here = 42
// 

I'm sure there are some other places where comments are good. Bill Sourour has a great article that I read after my first draft and I stole the Stack Overflow reference from him.

So the next time you type //, just ask yourself whether you really need that comment or could you perhaps make your code a little more readable?