Adam McNeillyThis 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.
