Comment Conundrum

I picked up a post on reddit recently criticizing this blog post. The post's title is fairly inflammatory: "Don't Waste Your Time Commenting Source Code". I'll pause a second and let that sink in.

Now, I do software for a living and I'm what you could, charitably, describe as a comment zealot. I'll comment anything and everything. With me it's almost like stream-of-consciousness coding. I'm not saying that my approach is the right approach because it does leave a lot of extra for others to read but it's just the way I like to do it. However, I will concede that there is no magic "appropriate" level of commentating.

On the other hand I know that no comments is wrong. I think we can all agree on that. No? Okay, so here's why: code is not as expressive as human language. This is by design. We trade expressiveness for reduced complexity and ambiguity. We do our best with expressive variable names and clear coding standards but sometimes it isn't enough.

Take, for example, the following snippet:

Message message = new Message(Message.Type.TERMINATE)
List attachments = Engine.getAttachmentList()
if(attachments.size() > 0) {
   Collections.sort(attachments);
   message.attachAll(attachments);
}
Service.send(message);

It is fairly easy to see what the code is doing, but not why it is being done. This, I believe, is the true purpose of commenting. The code above is expressive and straightforward and easy to parse. You may not know, however, what it does or what it means. Let's comment it and review it again.

//in this case we are sending a terminate message
//because the transaction has run too long
Message message = new Message(Message.Type.TERMINATE);
//we know from the API that a TERMINATE message
//can have at least one attachment
List attachments = Engine.getAttachmentList();
//if an empty list of attachments is attached
//the receiving service will crash. This is a
//defect that should be cleared up in the next API release
if(attachments.size() > 0) {
   //if the list of attachments is not sorted then only the
   //in-order attachments will be processed.  this should be
   //solved in the next API update as well.
   Collections.sort(attachments);
   //now it is safe to attach the list of attachments
   message.attachAll(attachments);
}
//use the service singleton to send the message
//instead of building another mechanism
Service.send(message);

That might be a bit verbose for some but I think it neatly captures the way that I feel about this sort of thing. I've added some why and context to the whole situation. This should help future humans understand more about the above code block and prevent them from removing the "Collections.sort()" because they want to save 3ms.

In an extreme example I'm a bit worried that the same guy who doesn't comment thinks he's a bit clever and comes up with something like the following snippet.

Message m = new Message(Message.Type.TERMINATE);
Service.send(Engine.getAttachmentList().size() > 0 ? m.attachAll(Engine.getAttachmentList()) : m);

Now, what's really cute about this example is that I've included a bug. Can you spot it? I'm sure you can but with all the cleverness impacted in there it isn't easy to parse. (You'd also only know it was there from reading the comments above anyway.)

So, let's look at the arguments presented in the blog post. The first is the old saw that well written code is self-documenting. I've already gone over this by example but basically only the what is documented. In complicated projects this is simply not sufficient.

There's an old story about a young girl who asks her mom a question, "Why do you cut the end off of the ham before you cook it?" and the mother replies, "because my mother did it that way." So the daughter asks her grandmother and receives the same answer. Fortunately, in this case, the great-grandmother is still alive and so the daughter asks her. Her answer is simply: "because I didn't own any pans big enough."

The 'why' of a thing is very important. All those years throwing away part of a ham...

The second argument he presents is that comments become obsolete. For everyone who espouses this argument: they become obsolete because we let them. They become obsolete because we're lazy and we don't update them. They become obsolete because we're not being diligent. This is our fault and not some inherent flaw in comments themselves. The author states that unit tests are better because if someone changes the code it will break the unit test thereby making the whole thing verifiable. Now, I agree, if comments could automagically verify like that it would be awesome.

So, overall, I see that the author of the post doesn't like comments for reasons that are directly traced back to humans. I think my take on this article isn't that we need to stop commenting (heaven forfend!) but instead that we need to get better at commenting.

I want to point out, as well, that the reddit post (linked at the start of this article) has some good criticism of this article as well so check that out as well. It gives me continued hope for my industry.

So, in closing, comment early and comment often.