This was presented at Droidcon NYC 2019. See the video here:
Earlier this month, there was a very controversial tweet by the official Java Twitter account, telling you to stop writing code comments.
It links to a Medium article which explains some of the problems with code comments and suggests the solution is to stop writing them entirely.
This is bad advice. I was shocked to see it come from the official social media account of such a popular programming language. Code comments can provide a lot of value to your code base and really help the next person who inherits your code. I also found the article to be unnecessarily harsh:
When you need to write a comment, it usually means that you have failed to write code that was expressive enough. You should feel a sharp pain in your stomach every time you write a comment.
I'm here to tell you that you are not a failure for writing comments. Nor that you should feel guilty for doing so. As a community, we shouldn't stop writing comments. We need to learn to write better comments. I'm hoping this will serve as a guide to help you do that.
The Risk Comments Pose
First, let's define a baseline understanding of the benefits comments bring us, but also the risks.
Comments are helpful because they can provide insight to the readers of your code that the code itself may not be able to.
Comments are a potential risk because they can become outdated. Changing the code doesn't guarantee we change the comments, and thus we could have comments that mislead us. That is why, by default, we should try to avoid comments.
However, we shouldn't avoid comments just for avoidance sake. We should make sure we're intentional about the comments we do - and don't - write.
In my experience, the code comments I've come across can be grouped into three buckets:
- This comment is unnecessary.
- This comment is unhelpful.
- This comment is helpful.
The goal is to make sure all of the comments we have are in the third bucket - helpful. We can start by removing any unnecessary comments, and then ensuring the remaining comments are helpful to the reader.
Avoiding Unnecessary Comments
Some code comments are just not needed. In these cases, we should just get rid of them. Otherwise there is more clutter in our files, and more risk of comments becoming out of date and actually going from unnecessary to unhelpful.
Remove Redundant Comments
The first type of unnecessary comment we can remove is a redundant one.
You may feel the urge to document a method and all of its params, but sometimes your code is so expressive it's hard to write a unique comment. That can lead you to something like this:
interface AccountDAO {
/**
* Inserts an account into the database.
*
* @param[account] The account that we're inserting.
* @return The ID of the inserted account.
*/
suspend fun insert(account: Account): Long
}
The documentation on the insert method isn't all that necessary. The first line provides a little info, but given the method name and the interface it belongs to one can deduce what it does. The explanation of the account parameter is again repeating something the code already tells me. The return statement, however, can provide some benefit.
So 2/3 of this comment is providing me with information that the code already provides me. That means it's redundant. I think that no comment is better than a redundant comment, because it doesn't put us at risk of having an outdated comment. So I would recommend only including the beneficial part:
interface AccountDAO {
/**
* @return The ID of the inserted account.
*/
suspend fun insert(account: Account): Long
}
An Exception
An exception to this rule is if you are building a public API or library for others to use. In this scenario, you should document everything that is available. Even still, we can be thoughtful about the comments we write and ensure that they are helpful, which we'll discuss later.
Change Code To Avoid Needing A Comment
Let's continue with the baseline goal to avoid comments. You've started writing one, and you've made sure it's not redundant. Next, you should see if you can rewrite the code to make this comment unnecessary, so that you can remove it.
Here is an example of a short one line comment that helps clarify what a method may do:
// Saves data to database
fun saveData() {
}
The author of this comment (spoiler: me) felt that comment helped clarify what the method was doing. In hindsight, I could avoid this comment entirely by just making a more expressive method name:
fun saveDataToDatabase() {
}
Another common example of this is when we write a method that does multiple things, and we want to break out the parts of that method, like this:
fun transferMoney(fromAccount: Account, toAccount: Account, amount: Double) {
// create withdrawal transaction and remove from fromAccount
// ...
// create deposit transaction and add from toAccount
// ...
}
Putting aside that having one method do multiple things breaks other best practices, this leads us to extra comments that we'd like to avoid. One way to avoid them is to just put the steps into their own methods that are appropriately named.
fun transferMoney(fromAccount: Account, toAccount: Account, amount: Double) {
withdrawMoney(fromAccount, amount)
depositMoney(toAccount, amount)
}
Writing Helpful Comments
If you've gone through the last section, and you still feel you should include your comment either because it is a public API or because you feel it provides additional value, we need to ask ourselves if it's going to be helpful to the reader.
Comments Tell You Why, Code Tells You What
I cannot remember where I first heard this quote, but it has always resonated with me. The code should tell you what is happening, but the comments can tell you why.
Let's look at a bad comment that I wrote recently, one that repeats the what:
/**
* A list of updated questions to be replaced in our list by an interceptor.
*/
private val updatedQuestions: MutableMap<Long, Question> = HashMap()
This comment is almost helpful, but I have to put it in the unhelpful category. It provides me a little extra info, but largely repeats what code can already tell me. In the last section we said we should remove these, but the story behind this one is that I actually want to provide insight into why I added this HashMap.
Let's turn this comment into a helpful one, by focusing on why:
/**
* The `PagedList` class from Android is backed by an immutable list. However, if the user answers
* a question locally, we want to update the display without having to fetch the data from the network again.
*
* To do that, we keep this local cache of questions that the user has answered during this app session,
* and later when we are building the list we can override questions with one from this list, if it exists,
* which is determined based on the key of this HashMap which is the question ID.
*/
private val updatedQuestions: MutableMap<Long, Question> = HashMap()
Now we have a comment that actually provides some insight into why I'm keeping a local HashMap, which is because I want to override an immutable list that I can't easily modify (thanks, Android). As a result, we have a helpful comment.
Comments With Examples Are Helpful
We keep mentioning that redundant comments should be avoided. A case where that is difficult is when we're creating a public API for others to use, and we want to make sure everything is documented. That poses a risk of some repetitive info. Here's another example:
class Pokedex {
/**
* Adds a pokemon to this Pokedex.
*
* @param[name] The name of the Pokemon.
* @param[number] The number of the Pokemon.
*/
fun addPokemon(name: String, number: Int) {
}
}
If we have to keep these comments, we can do our best to make sure they're helpful. One way to do that, if you can't provide additional info, is to provide examples:
class Pokedex {
/**
* Adds a pokemon to this Pokedex.
*
* @param[name] The name of the Pokemon (Bulbasaur, Ivysaur, Venusaur).
* @param[number] The number of the Pokemon (001, 002, 003).
*/
fun addPokemon(name: String, number: Int) {
}
}
This isn't drastically better, but rather than repeating information for our readers, we've now given them an example of what is expected.
Links To External Resources Can Be Helpful
We've all found solutions to our problems on StackOverflow. Sometimes that's an easy thing we can copy and paste into our project, but sometimes it might be a whole file or method that we reuse. In this case, it might be confusing to the reader where this code or concept came from, or why it's needed.
You can provide those insights by linking to the appropriate question or answer. Recently I created a programmatic ViewPager thanks to StackOverflow, and I wanted to give credit and make sure I had something to reference for more info, if the ViewPager ever had any issues:
/**
* A ViewPager that cannot be swiped by the user, but only controlled programatically.
*
* Inspiration: https://stackoverflow.com/a/9650884/3131147
*/
class NonSwipeableViewPager(context: Context, attrs: AttributeSet? = null) : ViewPager(context, attrs) {
}
Actionable Comments Are Good Comments
This might come as a shock, as comments are not usually presented as something that should be actionable. Actionable comments are helpful because you give the reader something to take away, preventing them from asking "what am I supposed to do with this information?"
There's two types of comments I think we can focus on as being actionable.
TODO Comments
In general, TODO comments are a big risk. We may see something that we want to do later so we drop a quick //TODO: Replace this method thinking we'll come back to it but never do.
If you're going to write a TODO comment, you should do one of two things:
1) Just Do Itβ’
2) Link to your external issue tracker.
There are valid use cases for a TODO comment. Perhaps you're working on a big feature and you want to make a pull request that only fixes part of it. You also want to call out some refactoring that still needs to be done, but that you'll fix in another PR. Link to something that holds you accountable, like a JIRA ticket:
//TODO: Consolidate both of these classes. AND-123
This is actionable because it forces us to go to our issue tracker and create a ticket. That is less likely to get lost than a code comment that will potentially never be seen again.
Deprecation Comments
Another time comments can be actionable is if you're leaving a comment around a method or class that you've deprecated and you want to tell the user where to go next.
I lifted an example of this straight from Android, where they deprecated CoordinatorLayout.Behavior in favor of CoordinatorLayout.AttachedBehavior:
/**
* ...
* @deprecated Use {@link AttachedBehavior} instead
*/
@Deprecated
public interface DefaultBehavior {
...
}
This comment was actually helpful because it not only tells me something was deprecated, but it tells me what replaced it.
Conclusion
While comments pose a risk, not all of them are bad. The important part is that you understand that risk and do your due diligence as a programmer to avoid that. Which comes down to:
- Removing comments you don't actually need.
- Rewriting code to remove comments if we can.
- Make sure the remaining comments are helpful.
- This is done by clarifying why, providing examples, or being actionable.
Have questions? Let me know in the comments! I'm also easy to reach on Twitter.
Top comments (23)
Consider the method:
sendNewsletterEmailOnSundayOkay that's long but you won't need a comment, let's shorten it.
sendNewsEmailSunBetter but can we do more?
emailSunNewsIt's getting harder to shorten and we are starting to loose context. This method could be mailing a customer on Sunday or maybe it's mailing the newspaper news on Sunday. So maybe a comment is needed... Or perhaps the problem is the method wasn't generic enough, bad design.
sendEmail(who, what, when)That way the implementation is clear.
Other things that might help, decorator annotations, type safety and so on.
I prefer
sendEmail(who: Person, what:string, when:Date)kind of thing. It's too verbose, but with clarity. But, we have our preferences in our code. Just, don't put dead code comments in that file.Why did you feel the need to shorten the original method?
Okay seriously, as a developer I feel a compulsive need to express more with less, but not so much less that I can't express my meaning.
In the same way anti comment opinions didn't want to see novels because it gets in the way or perhaps insults thier intelligence, I think it's the former. But most of all I knew this was a bad method, because generic methods should always exist first then helpers can derive from them, with comments explaining what the base method does.
I don't think it's a bad method. If you're only ever sending emails on Sunday, make a method that explains that. Making a generic (who, what, when) for only one use case is more complexity than you really need, IMHO. :) Premature optimization can be a bad thing.
Premature optimization doesn't exist in my book. Over engineering does. But if there are 7 days in a week and a client that might change thier mind. It doesn't hurt to keep it generic. That is where development point of view clashes because I do believe in future proofing but others don't, I get it and I'm cool with it, but I'm going to predict what is reasonable to predict, that's just me.
Somebody once told me "we don't know anything about anything. What we do know time will tell." So that is where my point of view comes from.
Yeah, and sometimes it makes sense because like you said there are 7 days.
But whenever a project manager tells me "we might want to support this" I think really carefully about how likely that is, and whether it's worth over engineering now or just building for the project at hand.
Of course, we are talking about work, work, that means delivery takes precedence and with haste I agree. You get a feeling when something will take too long to explore, that feeling is my limit.
At home I just ignore that feeling and make my scripts do backflips around the internet whistling to the tune of yanky doodle. Now that's over engineering to be proud of.
Peer pressure π
Comments are valuable when they enable me to skip over reading a section of code.
Yes, clear, expressive code is important, but sorry to burst egos- I don't want to read every line of even beautiful code. I'm a busy woman.
Write comments to summarize sections & reveal gotchas & important details. It's just more efficient that way.
Agreed. Maybe comments are TLDR; for code? π
The linked Medium article still raises a good point, that comments can be seen as an indicator for a special type of code smell. The one that puts a ton of cognitive load on you while trying to make sense of what's going on.
However, in that case the problem is not so much with comments but with the code itself.
I was recently working on an extremely convoluted spaghetti code base that didn't even bother with explaining the obscure if / else if / else if / else sections that spanned over hundreds of lines in a single method. I would have really appreciated comments that explained what was happening there.
I refactored that code and broke out each section that appeared to be doing one unit of work into separate methods (and some of them further down into more methods) with self-describing names. Along the way I found and fixed several serious bugs which were really hard to spot due to the convoluted nature of the original code.
The refactored result doesn't need any comments anymore. Everything is now split into single-purpose methods, and the original main method is now collapsed from several hundred lines to just about 7 method calls, each explaining what's happening. The main reason for that refactor was that I had to write unit-tests for that component. Breaking down complex logic to small single purpose units of work doesn't only make the code more readable but also gives you the extra benefit of easier, more controlled "unit-testability". I wrote for each refactored method a bunch of individual unit test. This would have been very difficult to do while everything was still crammed into the same method.
The Medium article also mentions that kind of refactor and presents it as an example why we should remove comments from code. However, in my case there were no comments in that inherited code base. I was simply dealing with crappy and undocumented code. I would have been extremely grateful for some comments explaining to me what each section did.
There are usually just a handful rare occasions when I decide to add comments to my code. It's when some kind of external system or dependency imposes some odd workarounds that are not obvious to understand, similar to your
HashMapexample.However, I do add (JavaDoc) comments as documentation to each type and method. I also try to make sure to provide them with examples, explain under which circumstances exceptions are thrown or under which circumstances
nullis returned, and so on.I like to generate documentation from these comments, and I find it quite sad and depressing when I only see type and method names but not a single word written in plain English. But that's my OCD speaking. Over the years I learned to tame my OCD a bit, so I also skip documenting simple getters and setters, where there is no point in explaining what's going on π
Great article!
Another way to address TODO comments is by incorporating a tool into your code review process that allows the team to prioritize and organize TODO comments. imdone does this by laying them out in a kanban board.
Neat, thanks for sharing that! I have also seen various static analysis tools that can actually fail a continuous integration process if you include TODO comments.
Unfortunately, the real world happens and TODO comments aren't bad. That's why I like suggestions like JIRA and (now) imdone because it helps create that accountability and making sure they aren't lost forever.
I think comments are very valuable for all the reasons you mentioned. As you said, I think the problem is we are just using them wrong.
Comments, when formatted and used with the right tools are also great for generating external documentation.
One thing that I like about the new release of Xcode 11 later in the year is that it as you update your code it can also help by trying to update the documentation for you, I'd like to see more tools start thinking this way.
Yes, Android Studio does this well too. If my comments reference a specific class or variable, refactoring that class or variable will update the comments as well. :) Really helpful in making public libraries that you want to generate documentation for.
I think it comes from blog.codinghorror.com/code-tells-y... which I wholeheartedly agree.
And you're right, going slightly verbose on comments on public interfaces of your API may only be allowed only if it's for documentation generation purpose. Libraries like Typedoc for Typescript (a little bit OOT here) generate documentations extracted from the comments, as other people may have mentioned.
Ahh it was definitely coding horror! Thanks!
My usual approach is to put comments in the spec file of a package to document the API of the package. If what a procedure/function does is quite obvious from the function and parameter names, maybe the comment reduces itself to one or two lines, just for the sake of automatic documentation extractors.
It is really unusual that I put comments in the body part, only if I am using some complex and counter-intuitive algorithms. Often I prefer to comment about the code status in a point by using "executable decorator" such as Assert, Loop_Invariant and contracts.
Sometimes I put a long (sometimes very long) comment at the beginning of the file where I explain what that specific package does and what is the "abstract model" behind it, abstract model that is somehow independent not only on the implementation details, but also on the API details.
An example: you have a package that implements the usual Employee class. In the context of your application an Employee can have a name, a serial number, a base monthly cost and it can be in several states (On_Vacation, At_Work, Ill, Goofing_Off, ...). Note that this model is independent on the API details (how are we going to change the cost? With a method Set_Salary? Or will it be a Apply_Increase?).
In my experience, having a conceptual model of what that package is trying to do helps a lot in understanding its API.
Depending on the language features it may not always be this easy, but that's a good idea. :)
I'm encountering an issue while trying to extend the functionality of the Chillwithkira Ticket Conversations widget. I need to add emails to the view, like in the activities formatter. The current server-side code (data.stream = $sp.getStream(data.table, data.sys_id)) only returns additional comments. Any suggestions on how to include emails in the ticket view? Thanks!