When to add comments

A lot of methods have comments, either explaining what they do, how they do it, why they do it. And a lot of times they are wrong. More often than not, you don’t need to add comments, so lets take a look at when and why comments are useful.

When we do add comments

A valid reason to add a comment may be one of the following:

1. It expresses something the language can’t
2. It is needed for certain tools
3. It is a non obvious work around.

It expresses something the language can’t

If we take the following method signature as an example:

function combine(array $input): string;

We don’t know what the input array is supposed to look like. Does it expect an array of integers, strings, certain objects? This is something we can’t express in PHP itself right now. But if we write it down like so, we get a better understanding:

/**
 * @param array<int> $input
 */
function combine(array $input): string;

Now you may be more used to seeing int[] as the type hint. the array<T> is the same. But the difference is that you can also say array<int,int> as a comment, which means that you expect the keys to be integers.

Another thing the language doesn’t support yet is union types, so a parameter taking an array or a string can’t be expressed yet, but with a comment we can say @param array|string. And our IDE and tooling will understand, as well as our colleagues (hopefully).

You may also be using a library like Doctrine Annotations to add even more functionality, through annotations.

It is needed for certain tools

Now i will preface this by saying that i am not a fan of littering your code base with comments that are used to instruct tools. For example @codeCoverageIgnore or @codingStandardsIgnore shouldn’t be in your code base, but they should be part of a config. However, with annotations we can support certain tools to help us with validating our code. For example, we can add great support for verifying immutability by using Psalm and the @psalm-immutable annotation.

It is a non obvious work around.

This is the ‘real’ reason you want to use comments. You may be doing something that makes very little sense, because you are facing a bug that you can not fix.

For example, a project i worked on could be used by using only the keyboard. However, we ran into a bug on firefox where a e.preventDefault() didn’t stop a dropdown from still being changed when an arrow key was pressed. The only way to do so was to disable the element. So that is what we did. But if someone would see the code, their first thought may be that it isn’t needed. (Because it isn’t on other browsers).

So we added a comment to the method:

/**
 * We temporary disable the selector, so the pressing of arrow keys doesn't change the rating given.
 * This is needed for firefox, as the event.preventDefault() does not disable the arrow keys on selectors.
 *
 * @see https://bugzilla.mozilla.org/show_bug.cgi?id=291082
 */
function temporaryDisableSelector()

Looking back this may not be the greatest comment. Part of it just copies the function name, but it does explain why it is there, and even has a link to a bug tracker.

When we don’t add comments

TL;DR: anything else.

A lot of comments fall in the following categories:

1. It duplicates the code
2. It duplicates VCS
3. It is wrong

It duplicates the code

Far too often i see comments like this:

/**
 * CheckoutController class
 *
 * @package \Website\Checkout
 */
class CheckoutController 

When this exact comment is on every single class in your code base, people ignore it. Now the one time you do actually have something important to say in your comment, no one is going to read it. Because everywhere else they can ignore those 5 lines, so why start reading them now.

Another one i see a lot looks something like this:

/**
 * @param int    $count
 * @param string $name
 * @param Order  $order
 * @param        $other
 */
function doTheMagic(int $count, string $name, Order $order, $other): bool

This comment does absolutely nothing, besides duplicating the method signature. Once again, if this is part of every doc comment, people will never read it, and when there is something there people will not see it.

Sometimes the comment may appear to be relevant, but they still aren’t:

$price = null;

foreach ($order as $order) {
  if($order->hasPrice()) {
    $price = $order->getPrice();
    // Stop after we find a price
    break;
  }
}

In this case, the comment just tells you what the break tells you. You are repeating the code in a comment. The danger here is that when your code updates, the comment may be wrong. But we don’t know for sure.

For example, a few months later it may look like this, and then our comment isn’t always true, and it is also in the wrong spot.

$price = 0;

foreach ($order as $order) {
  if($order->hasPrice()) {
    $price += $order->getPrice();
    // Stop after we find a price
    $price *= $order->getDiscount();

    if(!$this->combinePrices) {
      break;
    }
  }
}

It duplicates VCS

Far too many projects are filled with @author annotations, or the default PHPStorm annotations when you create a new file, where the creation date is in the comment. Those don’t add any value, as the information can be retrieved from your version control.

It is wrong

This is the danger with any comment. Generally comments don’t start out wrong, but the code around them updates. And the comment doesn’t get updated to match. I gave an example of this in the part about comments that duplicate code.

One i saw recently looked somethign like this:

/** 
 * Retrieves the information as follows:
 * 1. From the cache
 * 2. Otherwise from redis
 * 3. Otherwise it checks the config file
 * 4. From the database
 * 5. Or an empty string as a fallback
 */
function getQuickData(string $input): string
{
  return $this->sql->getFromInput($input) ?? 'Error';
}

Now looking back in the VCS, the comment wasn’t even correct when it was added, as the config file was never checked. But over time the method was refactored multiple times and the system changed, and now it just retrieves from the database.

But if we just check the comments when we peek at the method, we thing we may retrieve from cache, and we need to check for an empty string.

In conclusion

There are valid reasons for comments, but more then likely, your comments aren’t adding any value. And the more useless comments there are, the less likely the important comments are to stand out. So do consider removing comments that don’t add anything. Personally i am a fan of the Doctrine coding standard as one of its rules gets rid of useless comments for you.

Now there are more reasons to add, or not add comments to your code base. What reasons do you have for adding comments, or getting rid of them? And what are some examples of really good, or really bad comments you have seen? Do let me know in the comments.