Wednesday, April 14, 2010

What's in a comment?

One of the biggest surprises I had at my new job was finding out that writing code comments pretty much doesn't happen. It's not explicitly forbidden, but it is unofficially looked down upon by the developers.

Previously, I have always used code comments - Javadoc style method comments were more or less required (for the useless metric of code/comment ratio), and I would sprinkle others throughout the code as I thought was needed. I definitely don't want to fall into some sort of macho "you need comments?!" type of mentality, but if there is a better way of doing things I'm all ears.

Around my new office, the general thought on comments seems to be that if you need them you're doing something wrong. Also, we will often look at tests to see what any piece of code should do. Since we're using Jasmine (basically modeled after RSpec) each of our tests starts out more or less like a code comment anyway.

For example, say you saw this code:

MyApp.prototype.launch = function(action) {
  if (action == 'viewActivePages') {
    this.view.showActivePages();
  }
}

Reading this, you start wondering when launch would get called this way - there isn't any time you can think of where you would want to launch with this action. Then, looking at the tests you see this:

desc("#launch") {
  desc("when user taps dialog") {
    it("should show the active pages") {
      myApp.launch('viewActivePages');
      ...
    }
  }
}

Suddenly it all makes sense, you remember that the app can be launched by tapping on a dialog, and when that happens you should expect to see the active pages. The tests have successfully served as the comments.

This clarity could have also been accomplished with a comment:

MyApp.prototype.launch = function(action) {
  if (action == 'viewActivePages') { // tapping on dialog
    this.view.showActivePages();
  }
}

However, what if after looking at that comment you saw that the tests instead said this:

desc("#launch") {
  desc("when user taps dialog") {
    it("should show the user's work items") {
      myApp.launch('viewWorkItems');
      ...
    }
  }
}

Now you have conflicting information. The comment says that 'viewActivePages' should happen when a user taps on a dialog, but the tests say that 'viewWorkItems' should happen instead.

I drug you through the example above so you will have an idea of where I'm coming from on this. I don't know what the right answer is for whether or not to write code comments, but here is what I have figured out so far:

- Code comments, when they exist, should explain the why, code should explain the how
- Comments should be used when the code can't be made as clear as you would like (happens in CSS a lot)
- If you feel you have to write a ton of comments, you probably are doing it wrong
- Reading a comment is a lot faster than looking at a test
- An accurate comment can save you a lot of time

However, I also have found this to be true:

- Comments are often inaccurate, they get outdated very quickly and aren't well maintained
- An inaccurate comment is much worse than no comment at all
- Tests aren't perfect but are maintained much more than comments are

One place that I definitely think comments should be included is when the code can't convey all the information. For example, if you chose one algorithm over another because it performed better, it's probably worth putting in a comment to say so. The old slower algorithm won't be left around to show people reading your code that it was removed or explain why.

Another place that comments can be very useful is in things like CSS where you are very limited in how you code things. Conveying intent can be very useful when you come in later and try to figure out why something was done a certain way.

So what do you think? What is the right time to write comments? What should your threshold be for when warning bells go off when you find yourself wanting to write one?

No comments:

Post a Comment