I once posted  “comments indicate future refactoring.” I want to reaffirm my belief in that, and clearly explain. In order to fully appreciate this, let’s lay down some fundamental beliefs. These are beliefs I have built on experience, and later, I’ll apply them to more general fundamental beliefs like SOLID, but for now, these are simple rules I feel are good to follow.

  1. Do one thing.
  2. Do it well.
  3. Keep it concise.
  4. Use only what is given.

These aren’t overly complex rules. In fact, they are pretty simple. We are going to aim to meet these rules in a rather simple manner: using comments.

Now, let’s define what type of comments I’m referring to. I’m not talking about DocBlock comments. These are comments that allow for parsing by specific tools and allow us to generate documents about the code.

I’m also not referring to comments that repeat the code.

// increment a by 1

That is a poor comment, and should be removed. It does nothing except add to confusion.

The comments I’m referring to, however, are a bit more involved. But let’s start with an exercise.

Code should explain what it does. It doesn’t always do this. For example, what’s the purpose of the following code?

if ( preg_match(EMAIL_REGEX, $input) || 
     is_numeric($input) || 
     preg_match(USERNAME_REGEX, $input) ) { /* ... */ }

It’s easy to see what the code is doing, but not the purpose. If we add a comment, it will help matters.

// Check to see if the user input matches one of the searchable data sets
if ( preg_match(EMAIL_REGEX, $input) ||
     is_numeric($input) || 
     preg_match(USERNAME_REGEX, $input) ) { /* ... */ }

Hey! That helps. The code checks to see if the user input matches a pattern for searching a data set. Okay, that helps. But it’s still problematic. The comment isn’t clear. Is it referring to just the if-clause, or is it referring to the block inside the if-statement. Can you trust the comment? After all, as time progresses and code enters maintenance phase, things can and will change. Is it easy to read? It’s not difficult, but when you are read the comment, you then have to mentally associate that with the code. It takes extra effort. And for what? Saying something explicit.

Maybe it’s better that we ask a simple question: What is the comment trying to tell us?

If the input matches a searchable data set, run the blocked code.

Luckily, we can say that succinctly.

if ( $this->inputMatchesSearchableData( $input ) ) { /* ... */ }

// ...
function inputMatchesSearchableData ( $input )
    return preg_match(EMAIL_REGEX, $input) ||
           is_numeric($input) ||
           preg_match(USERNAME_REGEX, $input);

Using the extra method refactoring recipe, we’ve made our code much, much easier to read. inputMatchesSearchableData says exactly what the method does, and in our original block of code, it reads much easier than the comment did. We’ve also refactored the code that checks to see if the input is searchable. We’ve split responsibility, and suddenly, our original method does one less thing.

Every time I use a comment to explain what code is doing, I always ask myself if it’s code that should be removed. If what the code is doing isn’t directly related to the parent method, the answer is most likely yes.

Often times, the comment itself is a good indicator of the name of the function. Extract the method out, and keep each method small, precise, and simple.

2 thoughts on “Comments”

  1. I like the idea of questioning your code every time you feel the need to add a comment. I think we should question every line of code we write, but comments are often a great indicator of problems. 

    What do you think about going even further in the refactoring? I think I would add two more methods. Something like matchEmailFormat() and matchUsernameFormat(). It would make it easier to reuse, modify and read. If someone really needs to know how you check for it, he can go in the one line methods. 

    1. This is of course just the first step. You could add methods specifically for each of the checks.  This article, however, was merely focusing on using comments as an indicator.  Other indicators I will talk about are conditionals and loops.  The contents of an if or loop block is an indication of moving something into it’s own method.

      The idea is simple: when you do this, it should raise a flag.

      As you mention, we should question every line of code we write.  This hopefully helps to provide solutions to these questions.

Leave a Reply

Your email address will not be published. Required fields are marked *