The road to the programmer’s hell is paved with code comments
Many developers (and their managers) think that comments improve their code. I disagree. I believe that the vast majority of comments are just clutter that makes code harder to read and often leads to confusion and, in some cases, bugs.
A heavily commented code base can significantly slow down the development. Reading code requires developers to constantly filter out comments, which is mentally taxing. Over time, developers learn to ignore them to focus better on code. As a result, comments are not only not read, but also forgotten when related code changes.
Moreover, because all comments look similar, it is hard to distinguish between important and not-so-important comments.
Unhelpful comments
In my experience, unhelpful comments fall into a few categories.
Pointless comments
Pointless comments are comments that tell exactly what the code does. Here is an example:
// check if x is null
if (x == null)
These comments add no value but have a great potential to turn into plainly wrong comments. There is no harm in simply removing them.
Plainly wrong comments
My all-time favorite in this category is:
// always return true
return false;
The most common reason for these comments is to miss updating the comment when changing the code. But even if these comments were correct when written, most were not useful. Similarly to “pointless comments”, these comments should just go.
TODO/FIXME comments
When I see a // TODO: or a // FIXME: comment, I cannot help but check when they were added. Usually, I find it was years before.
Assuming these comments are still relevant, they only point to problems that were low priority from the very beginning. This might sound extreme, but these comments were written with the intention of addressing the problem “later”. Serious problems get fixed right away.
Let’s be honest - these low-priority issues are never going to get fixed. Over the years the product had many successful releases and the code might already be considered legacy, so there is no incentive to fix these problems.
Instead of using comments in the code to track issues, it is better to use an issue-tracking system. If an issue is deprioritized, it can be closed as “Won’t Fix” without leaving clutter in the code.
Comments linking to tasks/tickets
Linking to tasks or tickets from code is similar to the TODO/FIXME comments but there is a twist. Many tasks will be closed or even auto-closed due to inactivity, but no one will bother to remove the corresponding comments from the code. It could get even more problematic if the company changes its issue-tracking system. When this happens, these comments become completely irrelevant as the tasks referred to are hard or impossible to find.
Referencing tasks in the code is not needed - using an issue tracker is enough. Linking the task in the commit message when fixing an issue is not a bad idea though.
Commented out code
Checking in commented code doesn’t help anyone. It probably already doesn’t run as expected, lacks test coverage, and soon won’t compile. It makes reading the code annoying and can cause confusion when looked at if syntax coloring is not available.
Confusing or misleading comments
Sometimes comments make understanding the code harder. This might happen if the comment contradicts the code, because of a typo (my favorite: missing ‘not’), or due to poor wording. If the comment is important, it should be corrected. If not, it’s better to leave it out.
Garbage comments
Including a Batman ASCII Art Logo in the code for a chat app might be amusing but is completely unnecessary. And no, I didn’t make it up.
Another example comes from the “Code Complete” book. It shows an assembly source file with only one comment:
MOV AX, 723h ; R.I.P. L. V.B.
The comment baffled the team, as the engineer who wrote it had left. When they eventually had a chance to ask him, he explained that the comment was a tribute to Ludwig van Beethoven who died in 1827 which is 723h in hexadecimal notation.
Useful comments
While comments can often be replaced with more readable and better-structured code, there are situations when it is not possible. Sometimes, comments are the only way to explain why seemingly unnecessary code exists or was written unconventionally. Here is one example from a project I worked on a while ago:
// Do NOT remove this destructor. Letting the compiler generate and inline the default dtor may lead to
// undefined behavior since we are using an incomplete type. More details here: http://herbsutter.com/gotw/_100/
connection::~connection() = default;
This comment aims at preventing the removal of what might appear to be redundant code, which could lead to hard-to-understand and debug crashes.
Even the most readable code is often insufficient when working on multi-threaded applications. As the environment can change unpredictably in unintuitive ways, comments are the best way to explain the assumptions under which the code was written.
Finally, if you are working on frameworks, libraries, or APIs meant to be used outside of your team you may want to put comments on public types or methods. They are often used to generate documentation and can be shown to users in the IDE.
Credits: Memes for this week’s post sourced from https://programmerhumor.io/programming-memes
Top comments (5)
Good comments are even more difficult than good code
I used to write a lot of TODO / FIXME. When I realized that some of them were reaching 5 years old, I decided it was time to stop this technique. Now, when I see a TODO/FIXME I added that is more than one year old, I delete it .
One thought about "Comments linking to tasks/tickets". I believe that a comment just linking a ticket is not useful. On the contrary, I do believe that good comment ending with "for more details, see [JIRA ticket link]" can be interesting. If the link breaks, the comment is still useful. And as long as JIRA is up, the comment is kept relatively short.
If a JIRA ticket contains useful information then linking to it could be useful. I prefer linking to such tickets from commit messages but I admit it is less discoverable. If I decided to do it from the code I would provide a little context in the comment explaining why the reader needs to pay attention and copied the ticket # where they could find more details.
On my previous company (where we used Redmine and then Jira), we had configured our SVN (yes, sorry) to force commit message to start with a code that matches Redmine/JIRA ticket. Hence, all commits had links to tickets.
Wouldn't you then have to create meaningless tickets just to fulfill this requirement? I worked in a place that had a similar requirement and finding any useful information in tasks was difficult because the vast majority of them were basically empty and only contained the same title as the commit.
No, they were real tickets from the backlog. Some of them weren't really well written or very complete, but they described real bugs or requests for users.
Of course, some commits weren't linked to backlog items (typos, technical improvements, minor refactoring...)