Code Smells 2 – Comments

There are several adjectives that spring to mind when I think about code comments: Time-saving, Time-wasting, overly sparse, overly detailed, too-long, too-short, distracting, unobvious… Starting to see a pattern?

Comments are tricky because they’re very context specific. In some cases, a long and detailed comment is entirely appropriate. In others, its a nuisance. This isn’t helped by the subjective nature of advice about using comments. In this article I’ll be looking at both sides of the conflicting advice, and explaining how I reached my own set of guidelines for using comments effectively.

When should I use a comment?

When I first started university, I had a lecturer who insisted that every single line of our programs was commented? For example:

//Initialise y to 1
int y=1;
//Add 3 to y and store it in z
int z=y+3;

Even for this trivial example, you can see that I’ve spent far longer typing the comments than I have typing code. Have I gained anything by doing so? I don’t think so. Anyone with a basic understanding of programming languages can tell that I’m doing an assignment, some basic maths and another assignment.

In fact, my comments are actually hindering the understanding process by breaking my train of thought as I read the code. Not only that, but by stopping to write the comments, I forget what I’m supposed to be doing next. Another problem with comments is that, unless care is taken to maintain the comments with the code, they can quickly cease to have any relevance to the code they are supposed to be explaining. Clearly, this isn’t the way to do it.

The other extreme I’ve heard is not ever using comments. In general, I tend to more closely agree with this choice.

What is the function of a comment?

  1. To aid understanding of the code
  2. To highlight important and relevant information that may be of use to a developer in the future

So my answer to this question is when any of the following criteria are met:

  1. The intent of the code is not immediately obvious (more on this later)
  2. Certain conditions apply for this code to function correctly
  3. Modifying this code may cause a side effect that is not immediately obvious

Point 1 in the list above may indicate deeper problems in the design of your program. Points 2 & 3 may indicate problems in your environment or management and development process. Really, you should try to fix the cause rather than the symptom, but sometimes comments are unavoidable.

How can I avoid using comments?

Ideally, we want to avoid breaking our train of thought with useless, distracting additional information, but the important thing to remember is you can’t just stop using comments without having some framework in place to perform their function. Consider the following two code snippets:

//Calculate the total price of items in the shopping basket
public double calcSum() {
double s = 0;
//Iterate through the items in the basket
  for( Purchasable i : ib ) {
    // Update the sum total
    s += i.getVal();
  }
  return s;
}

and

public double calcSum() {
  double s = 0;
  for( Purchasable i : ib ) {
    s += i.getVal();
  }
  return s;
}

By removing the help provided by comments, we can see that the method’s purpose is to sum something, but we lose a lot of helpful information. We have failed to find an alternative method to provide context to our code. Now consider the following:

public double calculateTotalBasketPrice() {
  double totalSoFar = 0;
  for( Purchasable basketItem : basketItems ) {
    totalSoFar += i.getPrice();
  }
  return totalSoFar;
}

By using carefully chosen variable names that have a meaning in the domain we are applying them, comments are no longer necessary as the code is self explanatory.

A set of guidelines I have seen used very successfully for this are:

  1. A variable name should describe the data referenced by the variable, as closely as possible
  2. A variable name should never lie. It should always contain only the value its name implies, or nothing

I find the second point the most interesting. In the above example, I used “totalSoFar”. I could have used “total”. But, halfway through the loop, is “total” actually correct? does the variable contain the total price of all items in the basket? No, so the variable name is lying to us. In this example, the distinction is fairly trivial. In larger applications, however, being able to see at a glance what a variable contains at a given point in time can be a great time saver.

Useful and useless comments

Ideally, we want to avoid useless comments. Below are some examples of useless comments:

// Initialise y
y=1;

Well duh..

// This loops through the shopping basket and sums the price
for( Purchasable i : ib ) {
  sum += i.price()
}

Name your variables properly and this comment becomes an obvious restatement

// y+=3;

Never ever do this!

If I found this in a piece of code, my first thought would be: “Why did y need to be 3 lower when this was commented out last month? Was it commented out because its going to revert at some point in the future or did the other guy forget to remove it? Did this just get changed to another value?” Boom, train of thought gone. Confusion added.

If you want to comment something out for a quick test, fill your boots. Don’t leave it there to confuse the hell out of other developers later. If you really insist on leaving a block of commented code in, be kind and leave a note at the start saying why you didn’t just delete the whole lot straight away.

Useful comments are sometimes unavoidable (see above). Here are some examples of useful comments:

// The UK applies additional taxes to most items
if( user.Location == UK && !item.isTaxExempt() ) {
  finalCharge = ( basketTotal * UK_VAT_RATE )
}

If the developer doesn’t know about VAT, this code could be a little confusing. The comment provides minimal context to know this is a tax thing for a certain location. Suddenly the isTaxExempt() reference makes a lot more sense, and we know why we’re adding a seemingly arbitrary charge to our basket price.

// Unix environments tend to set this wrong, so set it explicitly
serviceRequest.setUserHomeDir( System.getProperty( "user.dir" ) )

Here, we know something that the next developer might not. Rather than letting him spend a week debugging it, we kindly tell him straight away that this line is redundant in all but one specific case

Summary

I try not to use comments if I can help it. I find that with careful naming of variables and methods, as well as adhering to commonly used idioms, design patterns and coding conventions, I can get the same level of expressivity without them. In doing so, I save myself some typing, I save the future developers some maintenance time and I keep the context of my code clear.

Following these guidelines also has the nice side effect of forcing me to write better, simpler code and really think about what I’m trying to achieve. That said, I also don’t have a problem sticking a comment in where I think someone would benefit from the additional information.

My answer isn’t necessarily the correct one, its just the one I find works best for me. Like many non-essential parts of any discipline, it really comes down to what you’re comfortable with, your own preferences and, at the end of the day, how far you’re willing to take it.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s