Don't Comment Bad Code - Rewrite It

Many teachings preach that it is a good practice to comment your code so that your buddies will know what the heck that code does or why it was written in such a way. There are many reasons why developers write code comments: copyright notices, to generate documentation, apologies for ugly hacks, explaining technical decisions, warnings of consequences, todos. I am willing to accept one or two of these in one way or another. But I firmly refuse to accept the most common reason to write a code comment – to explain what that code does.

Don’t comment bad code - rewrite it. – Brian W. Kernighan and P. J. Plaugher

Comments that explain what the code does

Some are saying that people who advocate against comments are just lazy enough to comment their code. I would say exactly the opposite and I meant it – people that write comments to explain their code are lazy enough to think more and refactor their code to be self-explaining. Unless you are writing code in Assembler you should not find a good reason to spend your time writing comments instead of spend your time writing good code. Modern programming languages are quite expressive if you name your classes, methods and variables well. Writing good comments can be harder than writing good code. We are no writers. We are programmers.

If your feel your code is too complex to understand without comments, your code is probably just bad. – Common Excuses Used To Comment Code by Sammy Larbi

Comments that explain what the intent was

I am far more inclined to accept comments that explain the intent or rationale behind a decision. I am far less inclined to accept comments that serve as an excuse for writing sloppy code. If you need to clarify why the code was written in a particular way you can always use a commit message. Do small meaningful commits with good commit messages and explain everything you think a future maintainer would be grateful to understand. Write a bigger commit message than the commit itself. People tend to forget. You may end up to be the one that benefits the most from these descriptions. Later on your colleagues can use git blame and read through the commits to see how the code evolved to its current state, and what were the coders thinking, what was their intent when they write it and what technical decisions were considered and why. It is by far a better practice than polluting your source code with comments.

Comments that warn the one who dares to change the code

# Dear maintainer:
#
# Once you are done trying to 'optimize' this routine,
# and have realized what a terrible mistake that was,
# please increment the following counter as a warning
# to the next guy:
#
# total_hours_wasted_here = 42

I often have a good laugh when reading through warning comments in code. They can be both funny and scary at the same time. Instead of taking your time to warn people just write a good test. Use a descriptive test framework to explain the weird use case behind that code. You can even use Cucumber if that will explain the use cases better. Anyone can just ignore your warning and change the code. No one can change the code without changing the test. And changing old passing tests stand out in code reviews. It will need a serious justification for doing that. Do some good to this wrecked world - write a test not a warning comment.

Pretty little liars

You have seen comments in popular IDEs or editors. They stand out nicely coloured and make the code look like a piece of art. The reality is that those are all just pretty little liars, wrong misleading comments that go far beyond any truth the longer they live. When working in a team you get different attitudes towards comments. Some people like to write frivolous comments, others are more dogmatic writing redundant noisy comments, wannabe geeks do not write comments at all, and so on. The bigger the team the bigger the differences in attitude and opinion. If you think you can enforce through code reviews a whole team to write comments in the same way and amend those comments as the code changes - well, good luck with that! The big trouble comes the moment the comment no longer matches the code. The problem now is that you do not know who is right and who is wrong. Is the comment right and there is a bug in the code? Or is the code right and someone has not updated the comment? Does the comment describes a valid use case or an obsolete feature? Now you are in a situation you do not want to be. The truth is - the older the comment is the more misleading it is. It is not realistic to expect even a small team to maintain each other’s comments.

Maintaining comments is expensive.

Let’s see an example

Bad code

Try to figure out what the code in the first code block below does without reading any further. Do not figure out the arithmetic calculations. They are quite obvious. Figure out what business logic could possibly be achieved through this code.

class TCalculator
  def initialize(amt)
    @amt = amt
  end

  def calc
    t = if @amt <= 18200
          0
        elsif @amt <= 37000
          @amt * 0.19
        else
          @amt * 0.325
        end
    t / 12
  end
end

Bad code explained with a comment

Bad code explained with a lengthy comment is still just bad code.

class TCalculator
  def initialize(amt)
    @amt = amt
  end

  # Calculates the monthly income tax from the salary. Salaries fall
  # into three ranges each of which has corresponding tax rate. If a
  # salary is below the low bracket then it is non taxable and the tax
  # rate is zero. If the salary is between the low bracket and the high
  # bracket the tax is 0.19. If the salary is above the high bracket
  # then the tax is 0.325. The monthly income tax is calculated by
  # dividing the income tax by the number of months.
  def calc
    t = if @amt <= 18200
          0
        elsif @amt <= 37000
          @amt * 0.19
        else
          @amt * 0.325
        end
    t / 12
  end
end

Good self-explaining code

Read through the code below. I bet you can get what the business logic is far better than reading any of the previous examples.

class TaxCalculator
  LOW_INCOME = 18_200
  HIGH_INCOME = 37_000
  LOW_RATE = 0.19
  HIGH_RATE = 0.325
  NUMBER_OF_MONTHS = 12

  def initialize(annual_salary)
    @annual_salary = annual_salary
  end

  def calculate_monthly_income_tax
    annual_income_tax / NUMBER_OF_MONTHS
  end

  private

  def annual_income_tax
    @annual_salary * tax_rate
  end

  def tax_rate
    return HIGH_RATE if high_income?
    return LOW_RATE if low_income?
    0
  end

  def high_income?
    @annual_salary >= HIGH_INCOME
  end

  def low_income?
    (LOW_INCOME...HIGH_INCOME).cover? @annual_salary
  end
end

Some of the refactoring patterns used to make the code more readable are:

Extract Variable

An explaining variable or constant is a named entity whose sole purpose is to describe the intend of that piece of code it was extracted from. Do not wait for an expression to become long and meaningless to extract it into a variable. Giving even a smaller expression a name makes the code more manageable and easier to understand.

Replace Temp with Query

Instead of extracting the expressions into local variables that make the methods longer, we used the Replace Temp with Query pattern to turn those temp locals into a reusable methods. It’s a win-win: we have named our expressions and the method who needs them is short and clean.

Extract Method

When there is an expression which can be explained with a descriptive and concise name, then extract the expression into a method. Do not write a comment that explains the expression. The method name will do the same job.

Readable code increases team efficiency by a factor

Programmers spend much more time reading code than writing code. Given a feature to implement you spend a lot of time reading the current code base, thinking about how the new functionality will fit into the existing code, making sure you are not introducing any regressions, configuring your environment, manual testing, debugging, bug fixing, etc. The actual typing of the new code is a really small part of that whole process. Much smaller than reading the current source code. So make yourself a favor: write code that is easier to read. Then you will spend less time reading it when the next feature comes in. Multiply that by the number of developers in your team and you will see how many hours of reading code you have saved from your team’s capacity by writing clean readable code. It is worth spending more time when writing the code to save others time when reading the code.

Explain yourself in code

Any fool can write code that a computer can understand. Good programmers write code that humans can understand. - Martin Fowler

You have class names, method names, variable names, argument names, constant names. So many names are there for you waiting to put them together into your poem of code. Do not comment bad code - rewrite it. Commenting code to explain what it does means that you have failed to express yourself in code.

References