Not All Code Comments Help: What Should You Avoid?

The debate over whether or not to comment your code rages on, showing no signs of stopping. ?Catalogers of anti-patterns include code comments as a code smell?because they explain problems in your code instead of fixing them. ?Not so, counter people on the other side of the aisle, sometimes angrily. ?No doubt the conversation becomes only more civil and even-keeled from there.

So-called religious wars like this emerge for a variety of reasons. ?Human nature causes us to get attached to our own decisions?and to assume people making different choices than us do so as a result of personality defects. ?But in the case of code comments, even more complicating factors exist. ?One person, when speaking about code comments, may mean inline explanatory comments. Another may mean?documentation for the sake of IntelliSense. ?People will argue angrily before realizing they’re not even talking about the same thing. ?Then folks come along and try to find a diplomatic, consensus-building middle ground.

Today, I’m going to do none of those things. ?I’m going to utterly punt on words of wisdom about how, when, and why you should comment. ?Instead, I’ll have a bit of fun by walking through something a lot of us can probably agree on. ?I’m going to talk about the sorts of comments that nobody should write, whatever your stance on the broader issue of commenting.

If you’re a general comment-hater, then of course you’ll agree that people shouldn’t write these comments because they shouldn’t write any. ?But even if you’re a comment enthusiast, you’ll concede that not everything that people ever put after two slashes (or a pound or whatever) deserves to be immortalized in your codebase.

Here are categories of comments that should not be.

Thanks, Captain Obvious!

Let’s start off with a simple example. ?I’ve never, in my life, seen anyone defend this. ?And yet, I’ve seen people do it plenty of times. ?I’m talking about the mind-numbingly obvious comment.

public void SendMailer(Customer customer)
{
    foreach (var address in customer.Addresses) //Cycle through all of the customer's addresses
        _mailService.SendGreting(address); //Call the mail service to send a greeting mailer to the address
}

This sort of thing usually comes from a good place. ?People internalize the wisdom to consider those coming after them and to err on the side of too much, rather than too little, information. ?But at some point, it just becomes pointless noise.

Maintenance programmers need context — not programming lessons. ?They know what foreach() does, and they understand the basics of calling a method. ?So save yourself some effort and the codebase some noise, and don’t bother explaining language basics with your comments.

End of Scope Comments

Have you ever seen people do this in code? ?At the end of a control flow block in code, they place a comment. ?For instance, let’s amend our previous example a bit:

public void SendMailer(Customer customer)
{
    foreach (var address in customer.Addresses)
    {
        _mailService.SendGreting(address);
    } //end foreach
}

“What’s wrong with that?!” ?I can feel some of your outrage radiating through the internet ether. ?”That can really help you understand where a gigantic loop ends!”

I’ll concede the point that such a comment can indeed help when you have gigantic loops with many levels of nested scope. ?In this case, the problem isn’t the comment, per se, but rather the maintenance nightmare that you’re using it on.

So forget these comments. ?They normalize this problematic coding style and make your team numb to it. ?Instead of commenting on end of scope, extract methods until you no longer have scope blocks so large that you can’t see where they started.

Source Control via Code Comments

Early in my career, I remember seeing (and, if I’m honest, writing) things like this:

/********************************************
* Author:      Erik Dietrich
* Method:      SendMailer
* Arguments:   Customer: the customer for whom to send the mailer
* Returns:     void
* Modifications
* 12/22/2016   Created
* 1/14/2017    Removed the mailer template parameter
* 1/17/2017    Refactored to use the mailer service
* 2/22/2017    Removed one line foreach brackets to conform with coding standard
* 4/4/2017     Adding code coverage
* *******************************************/
public void SendMailer(Customer customer)
{
    foreach (var address in customer.Addresses)
        _mailService.SendGreting(address);
}

Now, in my defense, Visual SourceSafe dominated the landscape early in my career. ?So source control wasn’t exactly a sophisticated and slick piece of tooling. ?But nevertheless, it existed.

Why am I talking about source control? ?Well, because tracking the history of a method in code comments needlessly recreates source control, and poorly at that. ?If people want to get context and understand a method’s history, your source control offers a much better and less error-prone way. ?Don’t use (abuse) comments this way.

The Dog Ate My Homework

Another sort of comment you see a lot is the excuse/apology comment. ?I’m sure you’ve seen something like this before. ?Somewhere in a source file, you see a block comment like the following.

Normally, we would never use global variables. ?It’s a bad practice, as everyone knows. ?But this past month, the business, in its infinite wisdom, decided that the report search module absolutely HAD to behave differently depending on who was logged in. ?We tried explaining to them that it would require a lot of architectural rework to allow that in a good way, and we tried to tell them about tech debt. ?But did they care? ?Of course not. ?So now we have this terrible implementation in our code, thanks to those morons in marketing.

Some people, including Jeff Atwood (to whom I linked earlier), hold that comments should explain “the why.” ?I understand that contention, but think it’s incomplete. ?The self-serving whining above theoretically explains “why,” but it’s pointless and worth avoiding. ?Comments explaining your rationale only make sense when you’re explaining something that a maintenance programmer has to understand in order to maintain the code. ?”I totally never normally code like this, but here’s the thing…” doesn’t qualify.

TODO

Speaking of intent-related noise, I’d love to retire the TODO comment once and for all. ?I would argue that this made some sense, once upon a time. ?Or at least, it was understandable. ?IDEs even started to pick these things up and aggregate them for you, so you could work this into your planning workflow.

But here’s the thing. ?We have tools explicitly made to track work items, tasks, etc. ?You can do it simply with Trello or with a more sophisticated ALM tool, like JIRA, Rally, Basecamp, etc. ?In fact, I’d be hard pressed to name all of your options. ?And these tools have also gotten really good at integrating cleverly with your code, providing traceability.

Just like you shouldn’t use your code for poor man’s version control, you also shouldn’t use it for poor man’s project management.

Don’t Touch This, Newbie!

I’ll close with the one that you may, perhaps, find the most controversial. ?I’m talking about the angrily-framed “here be dragons” comment.

You can imagine how this develops. ?There’s a block of code in the startup logic that initializes all of the various global variables and complex event structures in an incredibly delicate balance. ?Over the years, various new team members have wandered into this code, upset the balance, and caused the senior team members days of work tracking down the issue.

So what do they do? ?After a few frustrating instances, they guard the code with a giant, angry comment intended to put the fear of God into anyone happening by. ?I get it. ?But don’t do it.

Similar to the end of scope comment, this style of comment represents a terrible substitute for a real solution. ?If you have a delicate, dangerous block of code that confounds newbies and invites mistakes,?fix it. ?Or if you can’t fix it, write some form of automated test that checks for what you need, and fail the build if it fails. ?Both of those things?guarantee that no one can break things. ?Your angry comment offers no such guarantee, offering only a window into what people might take to be a toxic team dynamic.

Get the Most Out of Your Comments

The debate over whether or not to comment doesn’t figure to end anytime soon. ?Even among those who agree that you should comment, mini-debates rage about how and why. ?A big part of the reason for this results from context. ?It’s easy but incomplete to talk in abstract terms about whether or not to write comments. ?Someone saying “you can always find a way to refactor to self-documenting code” may not say that if he gets a look at what you’re looking at. ?Or the person claiming self-documenting code is a myth might not have seen your self-documenting code.

With this post, I don’t aim to address commenting in general. ?I’ll even concede that you might have a situation where a TODO comment or a “don’t touch” comment really are the most elegant solution. ?I can’t possibly say that such things are?never justified.

But I can say that these things tend to represent extremely suboptimal solutions to other problems. ?Code comments shine when they help maintenance programmers achieve quick understanding and clarity. ?But if you use them for all sorts of other things, you dilute their clarifying value and solve those other problems badly. ?So as you write your comments, make sure they’re pulling their weight.

Learn how GhostDoc can help to simplify your XML Comments, produce and maintain quality help documentation.

About the author

Erik Dietrich

Website

I am Erik Dietrich, founder of DaedTech LLC, programmer, architect, IT management consultant, author, and technologist.

This author does not have any more posts.