My mission to (almost) eliminate code comments
davejsaunders.com
davejsaunders.com
- When you have some "complex" technical code. Using natural language is sometimes the most effective way to explain what is done by the related code.
- When you deal with some business logic that can become weird an one point (process all these things in some ways, excepts these ones, weird edge cases, etc.). From a developer perspective, it does not always make sense, but it's always useful to have some background explaining why the code has been written in such way.
- When you have to consider history on a large code base. I know that people on HN mostly work on new shiny projects, but with legacy applications, you have to deal with all the bad/incorrect choices that were made before, and refactoring is not straightforward and is costly. I'm not saying it should not be done, but sometimes, you can work on it only in an incremental way. Comments allow you to add some perspective on the code (explaining the reverse engineering you've done, the initial assumptions, why they turned out to be incorrect, what should be done to improve the situation, etc).
Also, I disagree with the facts that comments are born out lazyness. "Lazy" developers don't bother writing comments. Actually, there don't care, and most of them do not even realise that at some point in time, someone will have to browse, read and understand their code. The only kind of comments I've seen from such people are "commented code", because "it might be useful" (it never does).
I see comments as a way to engage a (one way) conversation and provide useful and meaningful information to a future code maintainer. Sure, such comments should not clutter the code, be relevant and meaningful, stay up-to-date but it's - as many other things - a skill that require time to get it right.
Looking to eliminate all of those is not an approach that I think should be followed (but it's just my opinion). Focusing on allowing the code to be understandable by an hypothetical maintainer in the future is more relevant. If extra comments have to be added, so be it. These are tools, not ends.
Totally agreed here. There's some code in underscore that recursively copies strings as a way of writing '' because it happens to be fast. You'd think it was really weird if it wasn't marked as a performance optimisation.
> When you deal with some business logic that can become weird at one point (process all these things in some ways, excepts these ones, weird edge cases, etc.). From a developer perspective, it does not always make sense, but it's always useful to have some background explaining why the code has been written in such way.
That's the 'why' addressed explicitly in the article.
> Also, I disagree with the facts that comments are born out lazyness. "Lazy" developers don't bother writing comments. Actually, there don't care, and most of them do not even realise that at some point in time, someone will have to browse, read and understand their code.
> The only kind of comments I've seen from such people are "commented code", because "it might be useful" (it never does).
It sounds like you largely agree with the article. Save comments for 'why', you want to eliminate meaningless comments.
// Increment counter by 1.
counter++;
You should never do that unless you are studying programming and you have been required to explain every line of code along the way(though I don't know why a professor would ever want that, certainly no one ever asked me to do stuff like that).
As far as code that handles bizarre edge-cases, I tried to cover that in "Comments that explain why". Perhaps I missed the mark a little, but I think I was intending to agree with the point you made.
"Often, it’s because I’ve profiled it and found it to be significantly faster, or because it catches an edge-case that a more intuitive method of doing the same thing might not .. Comments like this get to stay."
To your point about "business needs this by Monday, 3 weeks ago". Under those circumstances, I write comments to explain my rushed code all the time.
In my personal case though, I'd consider this to be "Comments as an excuse not to refactor". It might be a good excuse (maybe the business need trumps readability in the short-term), but I know it's still an excuse and not something I should be doing all the time. I'm trying to get better at going back and refactoring those things later.
Document nothing until you have to do something weird and unexpected.
An example might be:
// NB: We're downsampling to 22khz because we found that the
// multi-speaker model chokes on higher sample rates.
source = downsample(source, 22000)
Or: // We don't perform internal retries here because our clients
// handle that themselves when we respond with a 429.
retryCount = 0
Or: /* Firefox renders this mistakenly, and overflows the
element. We opted to add padding to account for this
difference. See BUG-4282 for more details. */
padding: 0 0 3px;
This strategy captures all of the things that you'll forget, but leaves nothing that might go stale. In essence, only the things of value that couldn't be intuited from the code itself.Do not leave any other types of comments! Ever! Not even method docs. (The only exception being if you're writing a library and want to generate a documentation website).
You'll find that you start writing code that is like poetry. Structured, naturally intuitive, and easy for others to read. You'll become a better programmer, and your code base will improve dramatically. Comments, of course, will never go out of date.
Have you ever looked at a very large project(blue whale as I'd like to call them)? In 10 years time, 90% of the people who have worked on it initially will be gone from the company (or stopped contributing if it's an open source project). You are making everyone else's life a nightmare. To my mind the one project that got it bang on is postgres. It is by no means a library and the community's approach is perfect. Open up any of the source code files at random and you'll see what I'm on about. Just scrolling through the code and comments once will be enough for you to figure out exactly what is there and why.
> You are making everyone else's life a nightmare.
I work on code touched by tens of thousands of other engineers. In my experience, code without comments is typically structured, easy to read, and does exactly what is written. Code painted top to bottom with comments tends to be muddied, confusing, and more junior in nature.
The shape of solutions is common. You're not reinventing the wheel with every new project. Engineers are familiar with the design patterns, frameworks, and algorithms you use, so you don't need to force a context switch to point them out.
You can use the symbols themselves as a form of documentation. Instead of having an entire algorithm inline, individual queries or processing steps might be put into their own self-documenting methods:
verifyMarkdown(article_rating: &Rating)
upsertRating(article_rating: &Rating, author: &User, transaction: &Transactor)
publish(article_rating: &Rating, transaction: &Transactor)
enqueueSubscriberNotifications(author: &User)
Small pieces of code might not require a functions to wrap them, but by naming small units of behavior, you can communicate your algorithm step by step without comments.This is only one type of communication technique you can use. There are bountiful ways to improve your clarity.
I apologize if my examples are obtuse. It's late.
In the case of thousands, the likelihood of everyone leaving at the same time is much lower, but I'll go back to my postgres example, which I genuinely use as a benchmark as far as code goes. If everyone abandons it today for some reason, everything is so well thought out, well structured and documented that new people can jump in right away and the development won't be slowed down dramatically, granted that the new developers follow the current process.
Yes, I have. And almost none of the comments matched the code. My new "head of engineering" ruled that almost all lines of code must have matching comments. I now frequently find people changing the code but not the comments during code review.
- When you're adapting existing code, misleadingly named variable names and function names are going to be quickly noticed and fixed as developers are forced to read them (unlike reams of external docs or big comments). If you name a function "close" instead of "open" by accident, someone is going to notice fast.
> Well that's an organizational problem. Personally when I do code review and I see a comment which is redundant or doesn't match the code, I immediately return the PR and tell the developer to fix it.
You can justify any misguided developer practice like this though. The question is "is this a worthwhile thing to spend time enforcing?" and not "is it possible to enforce?". Getting rid of manual checks is a good thing when you can too.
This is turning into a philosophical question rather than a development one. What is worthwhile to begin with? As per Maslow's pyramid it's breathing, eating and drinking. I've had this argument with a former co-worker who was strictly against writing documentation and comments, precisely because it wasn't "worthwhile" and a "waste of time". "Right... Got anywhere to be right now or is your plan to finish your task till 11 and spend the rest of the day scrolling through facebook?" Common occurrence for that guy as a matter of fact. We've fought for years to take out the "we need that eod" to transition to long enough periods with realistic deadlines and plenty of time to spare for tests, documentation and so on. And it is something that we have now. It is proprietary software but the postgres example is perfectly valid - simply look at it.
Yes! And even then:
const int maxSampleRateForMultiSpeakerModel = 22000
source = downsample(source, maxSampleRateForMultiSpeakerModel);https://code.visualstudio.com/docs/editor/editingevolved#_pe...
I would also like to see unit tests unrolled like this for code reviews so others can actually follow the code. This would fight the tendency to accept highly abstract code just because nobody understands it.
I'm certainly not qualified to disagree with John Carmack! I can speak for my own experience, however.
I understand the argument that moving small sections of code into methods/functions hides the details. However, I probably spend more time reading code than writing it. When I'm in 'reading mode', I really just want to understand the control flow; I want the details to be hidden from me.
If I'm debugging code, or looking for performance improvements, then I do need to see that complexity. More function/method calls will make this slightly harder for me, but it's a trade-off I'm willing to make for the more common case, I think.
>In no way, shape, or form am I making a case that avoiding function calls alone directly helps performance.
Carmack argues that inlining (small) functions can reduce bugs in handling state by making the logic easier to follow and eliminating any possibility of others calling intermediary functions.
I personally think that almost all the abstractions we use are totally unnecessary and we should instead just code the solution top to bottom. This works especially well in array languages, when the name of a user defined function might very well be 2 times longer than the function itself. For example, no need to ever define a function called "flatten" to flatten a list if I can instead just write ",/".
Moreover, function definitions are bad because they wipe your working memory. It's better if everything is packed together, ideally as densely as possible.
If we did things differently, it would entirely be possible for an entire 5 man team's code output for the year be 1-10 pages of code. And it would do 10000x what the blub languages we use today do.
Seriously, though, I have seen these 'just code the solution top to bottom' go horribly awry. Like when it produces a multi-thousands of lines function where indentation is about 7 levels deep with lots of nested switch/case and so on. These things can become completely unreadable and unmaintainable. The danger of code duplication always was and still is that at some point something needs to be changed and it needs to be changed in 100 places. And then the various places where the duplication occurs start to diverge and things become truly unmaintainable.
- the theme is to move the comments in the code by using variable names and function names that tell a story. Not a bad idea by itself, but nothing special there either.
- moving a simple one-liner arithmetic operation to a function that is stored somewhere else is a complexity increase that is hard to justify. Every line of code can be hidden behind a function, is that productive, acceptable and lean?
- not trying to be picky, but for free shipping I would prefer to do shippingCharge = 0 instead of that because that is what you want to do.
And a question: if the comments are out of the code (where they are mostly contextual), what kind of documentation is left behind for the support team? In my world the support team has encounters with code written 10 years ago by people no longer around, they know high-level what the code is doing, but the devil (bugs) is in the details.
> "Every line of code can be hidden behind a function, is that productive, acceptable and lean?"
I've seen some advocate for that exact thing, but I certainly wouldn't go that far. I don't think we have to have one or the other, there's a middle ground in my opinion.
If the code requires a comment to explain it, then maybe a well-named method/function is a better option. If the code is obvious in its own right (looping over a collection to add 1 to everything, to use a dumb example) then I don't think abstracting it is justified.
I certainly don't have any objection to single-line functions to add readability per-se. (with the caveat that a compiler would produce the same result either way, or otherwise the performance of another function call isn't an concern. I'm talking purely about readability here)
If you're expecting non-developers to sometimes interact with your code or to help with troubleshooting, in-line comments will always be appreciated.
One argument against these kind of comments is that they aren't the right place to teach somebody a programming language. However I can definitely see cases where they are justified.
- comments, the more the better - design patterns, the more the better
Even if absolutely useless; the over-architecting just for over-architecting sake is quite... Not only does it take a lot of time to actually reuse the code (a lot/all of those patterns are conceived for reusing code in the first place but in reality it takes people a long time to get into them and the gains are not convincing aka I see people just plonking together a little microservice in a fraction of the time with Node and (virtually) no design patterns to replace (parts of) the java/c# monster that does the same thing just to make the deadline or to skip diving into the sea of blackness), but the comments really do not help at all; they are just there to have comments. And the design patterns (+ the way they are used) are considered standard (they are, but not for young people who have no degree and no experience applying them, or maybe even knowing they exist).
Still, without all that, the merge requests are being declined until you pop in random drivel 'for best practice'.
I am a fan of terse but readable code these days (it changes somewhat); the design patterns are more and time wasting (to me), especially when enforced in this way (but I think everyone will agree with that anyway).
orderCost = orderCost - shipping;
with ApplyFreeShipping();
Now you've violated referential transparency, introduced some unnecessary global and mutable state, and not really made anything clearer.Let's take his other example:
var outstanding = GetOutstandingOrders();
Into var allOutstandingOrders = GetOutstandingOrders();
Please for the love of god, no. Variable names should be short, and in my controversial opinion, as short as possible. Long variable names actually make code more difficult to understand, not easier. Rob Pike talks about it in the Practice of Programming, and it's a common tenant among APL/J/K programmers as well.Here's how I wish programs were structured:
1) Make your code as terse and dense as possible so you can see as much of it at one time as possible (see code written by Arthur Whitney). 2) Make your variable names abbrevations or acronyms, like instead of 'convertInt', call it 'ci'. 3) Comment every function and keep each function to only a couple lines. 4) Because your functions are so small and dense, you'll never need to modify them, only throw them away. 5) Bam, your comments and never out of date because the functions you're writing never need to be modified, only deleted.
Unfortunately, most programming languages don't give you sufficient power to do anything in a couple lines. Which brings up the question, why don't we switch to those that do? Honestly, J or K would be totally usable for the vast majority of programming problems.
I'm afraid I can't agree with making variable names as short as possible as a general rule, though.
I think that the length of the variable name should be linked to a) how long it lives for and b) the distance between the declaration and the usage.
For example, I'm personally OK with using 'i' for the index in a 'for' loop, but stand by longer names such as 'allOutstandingOrders' for variables that are used throughout a function, where the declaration isn't within a few lines of the usage.
"allOutstandingOrders" does not look to me like a long variable name.
> Make your code as terse and dense as possible
I really hope I never get to work with any code you've written.
> Make your variable names abbrevations or acronyms, like instead of 'convertInt', call it 'ci'
I'm starting to think this comment is satire now.
If I have to read the comments on a function or variable to figure out what it does then my productivity is going to be absolutely kneecapped if I'm a newcomer to your project.
Descriptive variable and function names make code easier to understand, not harder.
Why? I don't want to see as much code as possible. I want to see the code I'm interested in. If I can read 4 lines of code and understand what it does, I spend less time and energy than reading 20 lines of code and trying to reverse engineer it.
> 2) Make your variable names abbrevations or acronyms, like instead of 'convertInt', call it 'ci'.
That makes no sense, it's gross and has only drawbacks.
> 3) Comment every function and keep each function to only a couple lines.
So you need one or more comments for two lines of code?
> 4) Because your functions are so small and dense, you'll never need to modify them, only throw them away.
If you're only writing them to throw them away, why not optimize and never write them in the first place?
> 5) Bam, your comments and never out of date because the functions you're writing never need to be modified, only deleted.
This is sarcasm, right? I feel like I've been trolled.
var pendingOrders = getPendingOrders()Why in the world would you comment every function if you could just embed the comment into the code?
x@>#:'x sort order alphabetically in place
does not to me seem substantially different from order = alphaSort(order)
When the comment is embedded into the function, the same effect you described (~"because your functions are small, you can throw your comments away with the function "). These variable and function names need not stick around any more than K comments. Ideally, the name only exists for as long as it has utility, and if it has no utility, it is an anonymous function / never stored in a variable. If there is marginal utility in the name, then sure, use a single character placeholder `o=>sort(compareFn)`. As soon as you need to hold onto an idea of a noun (as in a larger mutating reference frame) or a verb (as in any function you want to both compose and reuse), then names become helpful handles for reasoning about the code over time and opportunities for reuse, `alphaSort=sort(alphaCompareFn); sortedOrders=alphaSort(rawOrders)`. If there's no opportunity for reuse or if the function is trivial, THEN eliminate the longer names.The harder part here is the middle area. With very concise code, you can see the whole thing, but with sufficient complexity, you may not be able to hold what "o" is at any given time in your head. With verbose code, the code will read like highly precise English and you are unlikely to be confused about what "alphaSortedOrders" means, but you might lose track of what the larger program is doing.
The real problems arise in the middle: is "orders" my sorted or unsorted orders? If it's not concise enough, you can't immediately see it, and if it's not verbose enough, you don't have a handle on what it is you are operating on.
---
Verbose code helps for transferomg "comments" across code boundaries, as when looking at the function signature when calling a function. I don't see how this is substantively longer than doing the same thing with a comment, and it provides additional utility for contextualizing chunks of code. When you change the "job" of the code, you change the variable name at the same time.
So while within some complex and mutating function you might want "rawSortedOrders", in some other context where you already know what you have, you strive for density.
finalizedOrders = rawSortedOrders.map(
o=>o.id=guid()
)
or finalizedOrders = rawOrders.pipe(
sort(localeCompare),
map(o=>o.id=guid())
)https://en.wikipedia.org/wiki/Argument_from_authority
But as short as possible doesn't mean:
> Make your variable names abbrevations or acronyms, like instead of 'convertInt', call it 'ci'.
'ci' has no meaning. It is not a possible name for 'convertInt' because 'ci' is not a name. I have to visit the function to determine what 'ci'-ing something does. In 'ci', in turn, I'll have to deal with more minified code.
if (customersFirstOrder) {
ApplyFreeShipping();
}
when really the `if` should be inside ApplyFreeShipping as it is part of its rules: when and how. We just 'enable' free shipping as it applies.Why isn't this the body of a `ProcessOutstandingOrders` method?
var outstanding = GetOutstandingOrders();
...
Process(outstanding);Then you're doing it wrong. Comments can be used to describe your thinking process, an algorithm, or explain esoteric code. I write comments with the target audience being people who inherit my code 5 years after I've left the project.
I'd argue that lack of comments on large projects may be an indication of an immature developer.
I... I think he knows that. I don't think he was ever going to say "The majority of comments I write are born out of lazy coding, so that's perfect."
The reason for writing this article is that I know I often do it wrong.
I'm trying hard to make sure that the comments I do write add value to future readers of the code (including me), and are not just be a by-product of another deficiency in my coding style (such as poor variable naming, to use one example).
The example is also a good example why type declarations are so helpful. Just by looking at the lines "var outstanding = GetOutstandingOrders()" or "var allOutstandingOrders = GetOutstandingOrders()", I could not tell whether the function is returning a list of outstanding orders or whether it has a side effect (reading the outstanding orders into a buffer) and returning a boolean indicating that there are outstanding orders.
Comments aren't a crutch to be used because of an impairment , they are helpful support in the face of the challenges of diverse technologies, poor documentation and difficult communication over time and space. Importantly they are integrated into the code base and are aimed at a very specific consumer - future programmers. Future programmers who may include you!
I had a mate who worked at a place where the code once it was checked in was ran through a processor that deleted comments!
My aim is just to write _less_ comments, so the ones I do write mean something and aren't just noise.
The example with the unneeded comments for conditionals and the one that explains "what" a function does is something I see even senior devs do all the time.
I think a lot of replies here aren't reading the article or are focusing on nitpicky examples.
"Comment your code!" is a deeply ingrained piece of advice people throw around without the warning that a lot of coders write comments in place of good function names and variable names.
However, that's not the only sort of code-reading. Comments can sometimes be better to get an overview of a complex program. Not if it is your own code, sure, or even in a code-base you work in. But faced with a huge amount of foreign code, over-commenting can be very useful (at least for me).
This should not be an excuse to write bad code - here I agree with the author. But given that, I see little downside in good comments.
It's an extremely challenging task to think about how others would perceive our coding. Perhaps the reader isn't familiar with a technique or function that seems obvious to you? It's the same as trying to give a lecture on a subject. Pedagogically, it's preferable to have more comments than less.
I like it, but I know some language communities don't.
if (customersFirstOrder) {
// Apply free shipping
orderCost = orderCost - shipping;
}
to if (customersFirstOrder) {
ApplyFreeShipping();
}
Does `ApplyFreeShipping` work on global state? How do we test it? At the very least it should be something like: orderCost = ApplyFreeShipping(orderCost, shipping);
which has advantages over the first version but is not necessarily better. The suggested version is just awful though.i.e. I have comments for type /* */ and // set to be grey on a slighgly darker grey so they are pretty much hidden (as they are useless most of the time).
I prefer the comments to stand out more than the code. Also helps eliminating useless filler comments since they are so noticeable. If you have comments, they should be important and helpful.
It definitely lowers cognitive load and “feels” better, but I still feel bad when adding function call overhead just for style (maybe the compiler is actually that smart and there’s no hit)
orderCost = orderCost - shipping; // Apply free shipping
Now it explains why.https://testing.googleblog.com/2017/07/code-health-to-commen...