29 June 2017

Comments are the Enemy

rant

Contrary to popular opinion and most forms of software development training, comments are not actually helpful. Comments are, in fact, the enemy. Adding comments to your code or your company’s code do not make you a hero. Comments are not to be admired. Adding a comment to your code is to admit failure. You failed to make your code readable.

So I Have a Problem with Comments

It’s not like I have never written a comment, its not like I delete them every time I see them and it’s not even that all comments are inherently bad. It’s just that when I see a comment that wasn’t necessary, either because it was already obvious what was happening or because the code could have been made obvious by better naming or the introduction of a well named method, I’m having to spend more time understanding this code than I actually needed to. I don’t like wasting time - my time is worth a lot, often literally.

There are different types of comment and different ways to avoid these comments and there are even some times when you absolutely should write comments. I believe its the existence of the latter that make people focus on comments so much, giving rise to the notion that we should comment everything, just so everything is definitely clear. Unfortunately, in the long term, this just leads to poor practice, unmaintainable code and everything generally being not clear.

Trivially Pointless Comments

The low-hanging fruit, if you will. This kind of comment generally looks like the developer wrote comments to reming themselves what to do, and then left them in. Each comment either just repeats the name of the method being called or about to be called. They’re basically pointless writing and even more pointless reading, you should remove them if you come across them, they’re just a waste of space and time.

An example from some Android code I may or may not have wrote in my youth. The comments can be safely removed without any loss to the readability (it would probably even help). Better yet, the comment and the lines of code can be replaced with a method, whose name is basically the comment. All the readability, none of the comments.

public void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    setContentView(R.layout.activity_main);
    // set title
    setTitle("App Name");
    // find buttons
    Button loginButton = (Button) findViewById(R.id.main_login_button);
    Button registerButton = (Button) findViewById(R.id.main_register_button);
    // style buttons
    loginButton.setBackground(Color.argb(255, 240, 30, 30));
    registerButton.setBackground(Color.argb(255, 210, 40, 40));
    // and so on and so on ...
}

Lying Comments

This is why I actually hate the trivially pointless comments. Gradual code changes can easily lead to drift between the code and the comment, especially because there’s no value in keeping the comment up to date. Eventually this drift leads to the comment lying about the code around it, making life harder for everyone.

An example highlighting my point, this time some Rails code, based on something I’ve seen before. The code was lying, suggesting that being removed (or having an owner) somehow meant subscriptions were applied. By tracing the git history that I was able to make sense of the code. At some point the denorms were made to be updated as part of a callback so the line that actually performed the update was no longer necessary and was removed, but the comment was left.

class Collection

  ...

  def add_item_to_collection(item)
    items << item
    item.collection_owner_id = owner.id

    # update denorms so subscriptions are immediately applied

    item.removed_item = false
    item.has_owner = true
  end

  ...
end

Explaining Poor Code

These are the kind of comments I mean when I say that writing a comment is admitting failure. Maybe it would be more kind to say ‘admitting defeat’ but I don’t feel like we should be kind to ourselves in this situation. Leaving a comment to explain our mess should leave us feeling bad. We should be using our valuable time to be be fixing your mess, not leaving a nice comment for the next poor sap that comes along.

We should not be spending time explaining how our magic number system works, we should be removing the use of magic numbers. We should not be crafting eloquent explanations of the parameters needed to make a particular method work, we should be introducing meaningful exceptions. We should be making the code better, not making excuses.

Of course, when we have tried and we have failed, which is going to happen some times, we do need to check-in what we found, explain not only what is, but what was tried and why it could not work. When the code review comes, our comment should need no further explanation and hopefully we get a pack on the back for our valiant efforts, we should be able to keep our heads held high, even in defeat.

Pointless comments, of course, should never be checked in and code review should point them out without mercy and should definitely include some mocking.

Check out Uncle Bob’s Clean Code for a more in depth reasoning about the evil of comments and other anti patterns and the ways to approach to fixing them.