All Articles

Comments are a Code Smell

As a software development consultant focusing on helping teams level up, I often find myself diving head-first into codebases that have unusually high technical debt. Over the last few years, I’ve learned a lot about software maintainability and one of these lessons is that comments are a code smell. A few years back, I wrote comments fairly often and I encouraged my colleagues to do the same. I’ve learned a lot about programming since then and as I’ve become a better programmer, I’ve learned something: if you feel that your code needs comments, you’re probably better off refactoring. A common failure I see is developers using comments to tell what they’re doing. Comments should illustrate why, not what. It’s reasonable to think that the next person to read the .js file you’re working on will be able to read JavaScript… Please don’t do this:

// loop over the object
const formDataKeys = Object.keys(formData);
formDataKeys.forEach((key, index)=> {
   ...
});

That comment does nothing other than distract us from what matters: The code! The next issue is one that has caused me some pretty serious pain (and cost my employers a lot of money): Code that “needs” heavy commenting is generally written by someone who doesn’t completely understand the problem and/or their solution. This can lead to comments that are inaccurate or misleading. Have you ever solved a defect after hours of debugging spaghetti code, only to learn that the comments were wrong about how things worked? If you haven’t, it’s about as fun as it sounds. The best way to avoid having this happen, is for developers to ask a TON of questions during planning meetings, and for the business to have answers to those questions. Solid requirements make it a lot easier to formulate a solution, which makes it a lot easier to write sensible code.

The last issue I’d like to touch on is the fact that comments can rot, just like code. For those unfamiliar with the term “code rot”, it refers to those bits of your codebase that slowly become hard to maintain, scary, and prone to failure. Much like the previous scenario, this situation can cause big maintenance nightmares although it’s root cause is much different. Instead of a programmer being wrong at the start, this is usually the result of an overworked (or sloppy) developer making a bug fix or extending code, and not updating the existing comments to reflect their changes.

As Uncle Bob says, “Code should read like well-written prose.” Often times, code comments are intended to provide clarity to hard-to-grasp code. I advocate that instead of trying to explain the mess we’ve made, we should strive to write code that needs no explanation. Now, to be clear, this doesn’t mean that I think my code’s better than yours and I’m too good for comments. I still write comments, but any time I feel the need I ask myself: Can I be more expressive with the code? More explicit? Here’s an example:

//check whether URL provided is valid
if(!req.query || !req.query.url || !isUrl(req.query.url)) {
    res.writeHead(400, {'content-type': 'application/json'});
    res.write(JSON.stringify({status: 400, message: "Invalid RSS feed URL"}));
    res.end();
    return;
}

This can be cleaned up a little bit:

let urlIsValid = (!!req.query.url && isUrl(req.query.url));

if (urlIsValid === false) {
    res.writeHead(400, {'content-type': 'application/json'});
    res.write(JSON.stringify({status: 400, message: "Invalid RSS feed URL"}));
    res.end();
    return;
}

The second version is a bit more verbose, but also much easier to understand. We’ve reduced the complexity of the expression that determines whether a URL is valid, stored the return value of that expression in a well-named variable, and then made a very explicit comparison in our conditional statement. By doing so, we removed a comment while improving the maintainability of our project. This code could still use some refactoring (like using the Express Response objects API rather than the low-level node .writeHead and .write methods) but that’s a topic for another blog post (and pull request). Before we get too crazy with refactoring, we’ll need a way to ensure our changes don’t introduce bugs so I don’t want to go any further without tests in place. By the way, having a solid test suite has a benefit that a lot of people don’t consider: It serves as documentation for developers, and further reduces the need for comments in your source code. Using BDD, you can even have tests that are so easy to read, your non-developer team members can use them as documentation, as well.

In a future blog post, I’ll cover how to add tests to legacy code so that we can refactor with confidence while documenting our code. Stay tuned to see how!

-Matt

Published 20 Apr 2017