How to Improve Code Comments with Little Refactorings

Shares

Does your code have comments? Sometimes they’re helpful. But most of the time…

Disclosure: The book links below are affiliate links. If you buy anything, I earn a commission, at no extra cost to you.

As Jeff Atwood explains, code tells you how, comments tell you why. A well-placed code comment is a level above the code itself, explaining why something is written the way it is. But! We can express most comments as code, using well-named identifiers. The Refactoring book calls this Introduce Explaining Variable. Martin Fowler has since renamed that refactoring to Extract Variable to help folks find it in their IDEs.

Code comments: Example from exercise

Let’s look at a specific example from my Code Smells exercise:

override func viewDidLoad() {
    super.viewDidLoad()

    if viewModel.isAlternateStyle {
        // Alternate style needs different placeholder text.
        textField1.placeholder = strings.field1Hint
        textField2.placeholder = strings.field2Hint
        setUpAlternateView()
    } else {
        textField1.placeholder = strings.field1Placeholder
        textField2.placeholder = strings.field2Placeholder
        setUpNormalView()
    }
}

First, I apologize for the awkward names. Many people said the generic names like textField1 and textField2 were a code smell. Normally, yes! But they’re so generic because I want to keep my job: I anonymized a section of real code I had encountered at work.

Mark Bernstein responded to the code smells exercise with many good observations, including:

The first smell is the comment; if part of the code needs explanation, it can be explained by making it a method.

In other words: extract and name. This might be as a variable or as a method, depending on how many lines we’re talking about. But give it a descriptive name.

Moving things around

How many lines are we talking about for this comment? It says,

// Alternate style needs different placeholder text.

Clearly this applies to the next two lines, which set the placeholder property for textField1 and textField2. But wait, they’re also set in the else clause. And the comment is talking about the difference.

What if we separated that if-else block into two? Like this:

override func viewDidLoad() {
    super.viewDidLoad()

    if viewModel.isAlternateStyle {
        // Alternate style needs different placeholder text.
        textField1.placeholder = strings.field1Hint
        textField2.placeholder = strings.field2Hint
    } else {
        textField1.placeholder = strings.field1Placeholder
        textField2.placeholder = strings.field2Placeholder
    }

    if viewModel.isAlternateStyle {
        setUpAlternateView()
    } else {
        setUpNormalView()
    }
}

This unifies the things that are the same (setting the placeholders for the text fields). It separates the things that are different (setting up the rest of the view to match the desired style).

“But now we’re repeating the if statement!” Sometimes, to clean things up, you first need to make it messier. If you’ve ever organized a messy closet, you know what this is like.

Now the comment could move up so it comes before the first if statement:

    // Alternate style needs different placeholder text.
    if viewModel.isAlternateStyle {
        textField1.placeholder = strings.field1Hint
        textField2.placeholder = strings.field2Hint
    } else {
        textField1.placeholder = strings.field1Placeholder
        textField2.placeholder = strings.field2Placeholder
    }

The code comment now lives in the right place: it attaches itself to the entire block. Try to make the scope of your comments obvious.

But at this point, it’s pretty clear that the comment isn’t adding any value. It’s just describing the code.

When you see a code comment that only describes the code, delete the comment.

Rules of thumb for code comments and refactoring

Let’s summarize what we’ve seen in this example. First, about code comments:

  1. Make the scope of your comment clear. You can do this for a single line by attaching the comment to the end of the line. For multiple lines, use blank space to separate the commented part from the rest of the code.
  2. Use your comment to explain why the code is necessary. Don’t explain the how; that’s what the code does.
  3. If a comment only repeats the code, delete it. ‘Nuff said.
  4. Can you transform the comment into an identifier? Use Extract Variable or Extract Method. Then you’ll have self-documenting code!

Finally, some observations about refactoring:

  1. If your IDE provides automated refactoring, use it.
  2. Bring similar things closer together. Move dissimilar things further apart.
  3. Don’t be afraid to make a slight mess while you clean, even if it means increasing duplication. Some things are hard to see until you move them around.

🛑 But before we do any refactoring, let me slam the brakes. We’re not ready to make any changes to the original code! There’s one big, big principle about refactoring we’ll look at… next time.

Search your codebase for comments. Do any have ambiguous scope? Can you change any into code? Can you simply delete any? Let us know how you did in the comments below…

About the Author Jon Reid

When I was a kid, programming was fun. But working in Silicon Valley, I saw poor code lead to fear, with real human costs. Searching for ways to make life better, I learned about Design Patterns, Refactoring, and Test Driven Development (TDD). Programming became fun again! I've now been doing TDD in Apple environments for 17 years. I'm committed to software crafting as a discipline, with the hope of raising us all to greater effectiveness and joy.

follow me on:
  • Josh Adams says:

    Great article. Here is another “why” comment I use: the URL of an OpenRadar or StackOverflow answer explaining the necessity or appropriateness of some hack.

  • Josh Brown says:

    The link to the Jeff Atwood article is broken — looks like it somehow got doubled. I think it should be: https://blog.codinghorror.com/code-tells-you-how-comments-tell-you-why/

  • I’d add another rule: remove any commented out code that you come across. It provides no value and can be incredibly confusing for someone coming to the codebase later on.

  • matias says:

    Thanks for the articule. I’d refactor viewDidLoad as (keeping same level of abstraction by removing those if to viewModel and setUpView function) :

    override func viewDidLoad() {
        super.viewDidLoad()
        textField1.placeholder = viewModel.field1Hint
        textField2.placeholder = viewModel.field2Hint
        
        setUpView(viewModel.isAlternateStyle)
    }
    
    func setUpView(isAlternateStyle: Bool) {
        if viewModel.isAlternateStyle {
            setUpAlternateView()
        } else {
            setUpNormalView()
        }
    }
    
  • >