Continuing my chapter-by-chapter review of A Philosophy of Software Design By John Ousterhout, today I’m going to review Chapters 12, 13 and 15. These chapters are all about the subject of comments, so I wanted to put the reviews all together. Chapter 14, which is not related to comments, will be discussed next time.
Chapter 12: Why Write Comments? The Four Excuses
John Ousterhout lists 4 reasons why developers opt not to write comments:
Good code is self-documenting
If that’s the case I guess there must not be a lot of “good code” in the world today. “self-documenting” seems a little bit like overkill, but I agree with this excuse to an extent: Good code should be obvious. It should be obvious to project developers what any individual class is or what any public method does. We don’t let people in off the street to come fiddle with our Github repos, so I think it’s a fair expectation that developers looking at our code are versed in the common pattern language and the Ubiquitous Language of our project. “obviousness” with that background may be a little different from code which os obvious to a more general audience.
I will agree with John that the “self-documenting” thing is usually rubbish, but I don’t agree that comments are necessarily the solution to the problem. This problem is overcome by improving adherence to the common pattern language and Ubiquitous Language of the project, and by simplifying code to make it readable. If it’s as quick to find and read the relevant code as it would be to find and read the relevant documentation, there’s no reason to do the later.
I don’t have time to write comments
I hear this excuse a lot with unit tests or profiling and performance tuning, or even code quality. People think that these things are a separate step in the development process, and they don’t have time to do more. Instead, doing things right includes all these things from the beginning and they aren’t separate steps. Writing comments or not writing comments isn’t a matter of time, because if comments are part of your requirement then the work isn’t done until the comments are it and should all be part of the process.
Comments get out of date and become misleading
Same thing goes with UML diagrams, almost immediately and universally. But I digress.
This is my single biggest complaint with comments. They do get forgotten and do become misleading. Tell me it doesn’t have to be that way all you want, I have seen it with my own eyes. Comments force you to have and maintain at least two descriptions of the system. If those descriptions disagree, even a little bit, it can be a source of serious problems.
And by the way, when the code and comments disagree, it is always the comments which are wrong.
All the comments I have seen are worthless
I mean, when the odds start approaching 100%, you might just be on to something…
Aside: Difference between comments and documentation
One thing that is missing here, I think, is the differentiation between in-code comments and user documentation. The former is for developers of the software, the later is for the users. The documentation should describe the technical details of the software in a way that the user can understand. Knowing who your users are is important, whether we’re talking about developers in downstream projects or we’re talking about DevOps engineers who are running and configuring your software, you should tailor the docs to the users.
Documentation is fine. I often find that users and downstream developers won’t read it and it ends up being a waste of time in hindsight, but it’s usually worth doing anyway. The documentation acts as a bit of an anchor, helping to reduce changes to the public API. This, in turn, helps promote backwards compatibility for the users. Win-win. For the rest of the discussion I won’t be talking about Documentation.
Comments, on the other hand, often are worthless. They’re written by the project developers for the project developers. If the code you’re modifying isn’t obvious, it needs to be rewritten until it is obvious. Per John’s own suggestion, we can encapsulate complexity into a relatively small number of locations. Most of the code will be simple and obvious and therefore not require comments. It’s only the few complicated bits where comments might be needed, but we don’t let just anybody monkey around in there. The developers working on the most complicated components are a selected group, so the comments they need may be quite different from the comments that would help a more general audience.
Comments as Code Smell
Maybe I’m just an anti-social cynic, but I always view comments as a code smell. Their presence indicates that the code isn’t clean or obvious enough, and that’s usually enough impetus for me to refactor something. Keep refactoring until the comments can be safely removed. Then remove them.
The overall idea behind comments is to capture information that was in the mind of the designer but couldn’t be represented in the code.
The code is the set of instructions actually executed by the computer. It is the behavior that is actually being performed. If an idea isn’t represented there, it doesn’t matter. And if it doesn’t really matter, why should we include it in our work? We can talk about my “intent” or “hidden knowedge” forever, but at the end of the day the actual behavior of the code matters and the intent does not. This takes us to…
Chapter 13: Comments Should Describe Things that Aren’t Obvious from the Code
In this chapter he really muddies the water between user-facing Documentation and developer-facing Comments. Again, I’ll only be talking about the Comments.
Don’t repeat the code
Unfortunately, many comments are not particularly helpful.
My thoughts exactly. Next John shows this code example:
/*
* The horizontal padding of each line in the text
*/
private static final int textHorizontalPadding = 4;
John says that we should have this comment instead:
/*
* The amount of blank space to leave on the left and
* right sides of each line of text, in pixels
*/
Sure, mentioning that the unit is “pixels” is more helpful than the first attempt, but it only helps people who are looking at the definition of the variable. Better than putting “pixels” in the comments would be to include it in the variable name:
private static final int textHorizontalPaddingPixels = 4;
I’m also a bit dubious of the idea that we need to spell out things like padding being “blank space”, or that the padding of a block of text would effect “each line of text”. Presumably a person working on a text editor would know that much already, it’s part of the ubiquitous language of the project. The only real help that a comment would add here is that the padding is appended to both the left and the right side of the text, but then I’m immediately drawn to wonder why we’re using a single variable for two separate concepts. The word “and” in the description is a dead giveaway that this value is doing too much. Just because two concepts happen to have the same value doesn’t mean they should be a single variable. De-duplicate. The code may serve two different masters, it may change for two different reasons, it should be two different variables:
private static final int textHorizontalPaddingPixelsLeft = 4;
private static final int textHorizontalPaddingPixelsRight = 4;
Now, with the fundamental knowledge about how text works that we expect from all our developers, plus improving the names of the variables, I think we’ve eliminated the need for comments entirely while also ending up with a better and more flexible design. Now we can treat left padding and right padding as separate values which is how, for example, every single word processor ever designed does it.
Aside: Units for Time
I was once working on a piece of code which interfaced with RabbitMQ. One of the methods had a number of parameters which represented spans of time: time-to-live for a message, expiration time for a queue, a timeout to wait for a response from the server, etc. All of the parameters were integers, but some were in measured in milliseconds and others in seconds, and the only way to know which was which was to read the documentation! The number of bugs caused by this was enormous. Adding a simple MS
or S
suffix to the variable names made it completely obvious to anybody using it what the units are supposed to be, and decreased the number of bugs without increasing documentation readership rates which remained, as always, near zero.
Lower-level comments add precision
John has this to say about comments:
Comments augment the code by providing information at a different level of detail
Then he shows this example of code and commentary:
// Current offset in resp Buffer
uint32_t offset;
it’s not clear what “current” means.
Why isn’t it clear? What’s wrong with the code that the notion of “current” offset isn’t obvious to the reader? If we were in a simple, single-purpose Iterator pattern class, for example, the notion of Current would be pretty clear, it’s part of the definition of the pattern. He improves the comment as so:
// Position in this buffer of the first object that hasn't
// been returned to the client.
uint32_t offset;
I’m not sure this comment is any better. We’ve changed “resp Buffer” to “this buffer”, which I guess is actual English (If “resp” was part of the ubiquitous language of the project, the developer should be familiar enough with that word). But, what if instead of the comment we just changed the name of the variable:
uint32_t firstObjectNotYetReturnedOffset;
Not only have we included all the information from the comment (except the, likely unnecessary, mention of “this buffer”, which we could probably figure out when there’s only one Buffer reference in the code) but now that information will be visible everywhere in the code the variable is used, not just at the point of declaration. An even better, more succinct name might be, in the appropriate circumstances:
uint32_t nextOffset;
It’s worth pointing out that these examples are really too small and too simple to completely prove the point, but I hope I’ve at least shown that conveying important information without comments is at least possible if you put some thought into it.
Higher-level comments enhance intuition
I don’t have much to add on this point, comments can help to give the high-level overview of very low-level code. But then again, you’d only use these kinds of comments in places where you’re really in the weeds of low-level details and, if you’ve encapsulated your complexity away to few places, the opportunities to use these comments are few.
I think the only comments in the entire CastIron project are these kinds of comments in the most complicated portions and, as I mentioned before, these often serve as a code smell to encourage me to refactor to a simpler and more understandable design over time.
Chapter 15: Write The Comments First
Many Developers put off writing documentation until the end of the development process, after coding and unit testing are complete. This is one of the surest ways to produce poor quality documentation.
Same thing goes with unit testing and, often, consideration of performance (premature optimization may be evil, but being completely unaware of performance characteristics and the basic size of data is conduct unbecoming a decent programmer.). As I’ve said before elsewhere, doing programming right doesn’t mean you write the code, then you write the tests, then you do whatever else is required, because the impetus to say the project is “done” after the code but before all the other bits are complete is too high. The work isn’t “done” until every part of it is done.
I use a different approach to writing comments, where I write the comments at the very beginning
I follow a similar process, sometimes, for writing an important method. First I write a checklist of tasks the method needs to do, and I delete them when the code to implement each step is complete. That way I know what the code needs to do as I’m creating the implementation, and when all the comments are gone I know my work is complete.
public void UpdateRecord(long id, Action<Record> update)
{
// Validate the arguments
// Load the data from the DB
// Assert that the record exists or throw a not found
// Perform the requested update
// Validate the record before saving
// Save to the DB
}
Then I’ll replace each line of comment with the implementation. The implementation for each line of comment might be one line of code or many, or maybe zero if the task is taken care of elsewhere
public void UpdateRecord(long id, Action<Record> update)
{
Assert.IsValidLookingId(id, nameof(id));
Assert.ArgumentNotNull(update, nameof(update));
var record = _recordStore.Load(id).AssertFoundOrThrow();
update(record);
_recordValidator.ValidateOrThrow(record);
_recordStore.Update(record);
}
This is a useful technique when the code I’m trying to write is complicated and it’s hard to keep all the steps in mind at once, or when I anticipate a lot of distractions and want to make sure nothing important gets lost in the shuffle.
John starts with class-level and public method comments first to help him solidify the interface, then he uses a similar strategy as I use for writing the method bodies (he, of course, doesn’t delete the comments when he’s done coding). Basically, it’s the red/green process espoused by Test Driven Design: Write the comments first and then write code until the comments are correct. I’ve heard this called “Documentation Driven Design” before, but I’m not sure that term has gained any kind of wide-spread popularity.
Comments serve as a canary in the coal mine of complexity. If a method or variable requires a long comment, it is a red flag that you don’t have a good abstraction.
He seems to agree with me, in principal if not in degree, that too much comments are a code smell. Define “too much” as “anything that isn’t zero” and you have my general position on the matter.
You can compare a method’s interface comment with the implementation to get a sense of how deep the method is: if the interface comment must describe all the major features of the implementation, then the method is shallow.
This, actually, is a useful heuristic, and another good point in the column of writing interface Documentation. If you can’t describe what a method does without also having to describe how it works, chances are good there’s something wrong with your abstraction or something missing from your understanding of what the calling code is really looking for. One situation where this rule doesn’t hold is when you’re picking between two implementations which have similar behaviors but different performance characteristics. Using an array versus a Linked List, for example, where the basic operations are the same but performance differs wildly for some operations. Besides that, you really shouldn’t ever have to worry what is inside the black box.
I’m looking for the design that can be expressed completely and clearly in the fewest words.
This is another nice heuristic.
Conclusion
Maybe my dislike of comments is based on past trauma, or maybe it’s just my general preference to read code over reading English. I like the idea of a single source of truth. Between comments and code, it’s the code which always must be the truth. If there’s any disagreement about what the software does, the code is right and the comments are wrong. I won’t harp on this point any further.
I view code as being very fluid. It can and should change often, to satisfy changing requirements and meet the changing needs of the users. This is my starting assumption. Under this assumption, it becomes clear to me that code modules should be small so the rewrites can be cheap, and comments serve only to slow the natural progression of the code: Every change to functionality would require changing two descriptions of the behavior. And then there is the relentless, iterative refactoring which serves to get things even further out of whack!
If you’re questioning whether to add comments to your code and how much of them, I suggest you first cleanup your code to reduce the number of comments you might need and then do it again. Eventually, you may find, that the number of comments required has become quite low indeed.