logo

Comment-pocalypse | Part 1

There are quite a few conventions when it comes to commenting your code. Let's break them down and see how good/bad they are.

First is the comment all your methods convention. Why would you do that? So everyone knows what your code does and how to use it. The upsides are that:

  • badly naming methods becomes less problematic
  • hard to understand methods are usually easier to understand
  • in case of examples, it is really easy to learn how to use

This all sounds fine and dandy but commenting all your methods is a double-edged sword. Let's look at an example:

/** * Gets all the hardware of the type and brand given. * Searches for substring brand inside each hardware's brand name. * @param type the type of hardware * @param brand the brand to search for * @returns a list of hardware * Example: getHardwareOfTypeAndBrand(HardwareType.Keyboard, "Das") * returns List with dasKeyboard1 and dasMechKeyboard2 */ public List<Hardware> getHardwareOfTypeAndBrand(HardwareType type, String brand) { ... }

I think we can all agree that having comments and examples at least looks nice. But what about their usability? Does the comment:

* Gets all the hardware of the type and brand given. * @param type the type of hardware * @param brand the brand to search for * @returns a list of hardware

tell us more about the method than:

public List<Hardware> getHardwareOfTypeAndBrand(HardwareType type, String brand)

The answer: No. And here we get to the first issue: REDUNDANCY. And you know what happens when you have redundancy in your code? When one change occurs in one place you will have to change it in another.

Same goes for comments. If we were to update our method and add another parameter we would also have to change our comment, otherwise our comment becomes MISLEADING.

Misleading comments are our second issue. Especially when we also have examples. But don't we have tools and plugins that help us updating our comments when the signature of the method changes? Yes we do, but that means more time investment in something that, at the end of the day, will be redundant.

Imagine changing our method from, instead of searching for brands that contain the given string we would check if the brands name corresponds exactly.

You should also change the comment. But would you, when it's 8 PM and in 10 minutes you have a deploy to make? Couple that with there being no way of checking if the example is valid and you have got a recipe for disaster. This is where this comment:

Searches for substring *brand* inside each hardware's brand name.

can also become misleading.