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.
I’ve seen commented-out code in merely every project I’ve worked on. And every single time I am going nuts about it. How am I supposed to deal with it? Is it still relevant? Leave it? Delete it? Who may ever need this snippet and when? Everyone assumes that someone else might need it so no one will delete it. At the end that code simply stays there forever and becomes less and less relevant with each day. It may even refer to collaborators that are long dead. It may be part of business logic that has changed fundamentally. It pollutes the rest of the code and distracts developers as everyone who tries to understand the rest of the class inevitably sees this commented piece and wastes time trying to understand its role. Commented-out code is evil. My advice: kill on sight. Whenever you see a commented-out code just delete it. You have your change under source control. So no need to worry about it. Anyone that might need it would grep through commits history, find it and use it.
Let’s see an example
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:
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.
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.
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.
- The Elements of Programming Style, 2nd. ed., by Kernighan and Plaugher
- Clean Code: A Handbook of Agile Software Craftsmanship, 1st ed., by Robert C. Martin
- Refactoring: Improving the Design of Existing Code, by Martin Fowler
- Hunting for great names in programming by DHH
- Every line of code is always documented