However, it does give me some comfort. When it’s not gamed, do other HNers also feel that a high comment:code ratio probably indicates quality?
There are reasons why this may be the case. (More thought, more time and a large team etc)
I don’t advocate using this measure to reward anyone because it would be gamed immediately.
Core, critical plumbing/logic at the kernel of business critical, long-lived applications, will be the source of my stress-dreams long into the twilight years of my life; in the form of a lack of documentation and a presence of organic growth.
To criticize myself quite bluntly: If the core code I worked on at work looked like this, I'd feel a great deal more comfortable in some of the changes/digging that inevitably arises.
I would never use it as an absolute metric; but I'd use the level of comfort e.g. a new dev feels when looking at something that might otherwise be a spiderweb and saying "Oh this makes sense" (As I do when looking at OP) as a north star for the most sensitive bits of logic.
It is in no way a guarantee that we got them all, but after spending so much time reason in through why those 'else' clauses were correctly empty, we thought it would be rude not to write it down.
In truth it was as much for future-me as anyone. My memory is know. To be spotty. :)
On the other hand, as soon as someone not safety minded gets their hands on it, trouble happens. Comments aren't updated (and there's no way to make sure they are checked, other than GREAT code reviews by the original authors, usually with at least two or three people doing critical reviews). Then the comments can become misleading and a liability as people will take them for truth, as they should.
If you have code that has a lot of subtle dependencies or edge cases, really great comments can help enormously.
A high comment to code ratio, where the comments are of the form "Do this thing in this way," indicates a lack of quality - generally a sign that the programmer is not confident enough in the language that they're writing in, and is trying to solve language-level problems instead of business-level problems.
Uncommented code better come with some reference for why the code exists in the form it does. Sometimes commit logs and the VCS "annotate"/"blame" feature works. Sometimes commit logs link to bug trackers or feature requests. Sometimes there's a README. If you don't have any of those, I tend to find that it's generally low-quality code.
Our purpose is to deliver business value. (Or non-business value, as the case may be; if you're writing a free video game for fun, you want people to successfully have fun.) Our purpose is not to generate lines of code. All code is, to some extent, legacy code; comments can help it be manageable legacy code, or make it even more unmanageable.
There are three kinds of comments: why, what, and how. How comments are almost always a sign that the design is poor or the complexity is too clever. Why comments are necessary to understand the code and are almost always a good thing. What comments can be useful guideposts for skimming code, but they are also extremely prone to code rot. I suspect what comments generally end up being neutral in a net value proposition.
You want a high ratio of why comments to code, but I suspect most high comment-to-code ratios arise from what comments, which severely attenuates the utility of a pure comment-to-code ratio.
I've been on Rails/React teams where comments were seen seen as a possible smell. Not talking about useless literal comments, just that their need was seen as pointing to possible bad design and that a well factored codebase was self-documenting -- ie. if you had to comment something, perhaps methods/vars were poorly named, SOLID principles were not adhered to, methods needed to be broken out, or it was just a sloppy approach. Even explaining design decisions was considered more in the domain of git messages and having nicely packaged atomic commits.
While I see that aspect of it, there's no getting around the constraints of the real world and that some problems are just difficult and much easier to grok with a user guide in plain english, so to speak. And mission critical stuff needs as many safeguards as possible.
That said, inaccurate comments can be dangerous and when your code is highly commented there is real danger things can get out of sync. If you're working on a 5000 line file that 100 developers have touched over a 20 year period... and no one has taken it upon themselves to do a recent comment audit, there be dragons.
I think this provides me higher quality, less bug ridden results. So if others use comments in this style I would tend to believe it does increase code quality.
If a line of code doesn’t match the comment, something is clearly wrong. ;)
And then there's something I recall running into, a decade ago:
using namespace std; // using namespace standardIt wasn't even "using standard namespace"...
And if your code is clean, you shouldn’t have a bunch of redundant comments explaining the obvious.
I absolutely do not comment enough, but knowing this, I try to stick to the principle that if I have had to stop and think through an expression before I write it, then I am likely to eventually thank myself for leaving a short explanatory note.
And moreover, it may not be me scratching my head over that nest of ternaries in a year's time - it may be some other poor soul. And while that poor soul won't thank me for leaving a comment, he or she will certainly curse my name - quite possibly vocally and publicly - for not leaving one.
There’s nothing worse than going back to a codebase from a year ago and seeing a couple magic numbers and having no clue how they came to be, haha.
If you're doing something obvious, you should generally be able to program it concisely, in which case you have a high comment-to-code ratio because the amount of code is low. Sometimes this will be because you're importing an external library to do something, or because you're calling out to an internal library. Sometimes this will be because you found a straightforward implementation. If you're finding yourself writing hundreds of lines of code to do a single obvious task then chances are high you're implementing it poorly (and, specifically, in a way where your defect rate is likely proportional to the number of lines of code).
And if you're doing several obvious things, then the point of the code is not to explain what the code is doing, but why it's doing that. What is the business purpose of the code? Which customer cares about this edge case that you're handling, and under what circumstances can you stop handling it? Why did you decide that the common library wouldn't actually work here? If you're converting data from an awful legacy format, why are your ingesters / parsers for the legacy format designed in this way? If you're micro-optimizing for performance, why are the optimizations sound (i.e., why do they accomplish the same thing as the unoptimized version), how do they work, and why did you decide these spots need to be optimized? Each individual thing you do might be obvious on its own, but the arrangement of the whole thing needs comments for each step, which again gives you a high comment-to-code ratio.
I just meant simple to understand variable, function, and class names. That combined with small classes and functions, makes following the logic of your program extremely easy.
Following concepts like DRY (don't repeat yourself) and the single responsibility principle ensure that you're making more easily testable code, and I'm sure less overall LOC.
// The binding is two-step process. PV.Spec.ClaimRef is modified first and
// PVC.Spec.VolumeName second. At any point of this transaction, the PV or PVC
// can be modified by user or other controller or completely deleted. Also,
// two (or more) controllers may try to bind different volumes to different
// claims at the same time. The controller must recover from any conflicts
// that may arise from these conditions.
As for redundant comments explaining the obvious, that seems to be something of a straw man, at least in my experience - personally, I have very rarely seen such code. The person who is not motivated to write useful comments apparently prefers to write no comments rather than useless ones.
Additionally, in no way am I advocating for no comments, that's obviously not possible (like your example). Comments are useful, even necessary, for code that might have an otherwise confusing logic to them.
I've seen plenty of code with documentation for a method with nothing more than:
/**
* Bills the user
*
* @param user The user to bill
*/
public void billUser(User user) {
//
}
With regard to the sample, then if that is your experience, I cannot deny it, but, irritating as it may be, it seems fairly harmless. It appears to date from a time when it was thought that extremely prescriptive coding style standards was the way to fix programming - an even less realistic belief than the idea that code can be entirely self-documenting.
I see this as someone trying to fool a linter that demands they have documentation. I think it's better to say "comment everything" because it puts documentation as a first-class consideration rather than an afterthought.
https://github.com/miohtama/aliens-vs-predator/blob/master/s...
Comments are very barebone. There is structure, but needing to mess with this kind of code would be scary. Granted, most games are write once and never look back.
// It might look like you should do X here but esoteric reason Y dictates that you should do this instead.
It is absolutely useful to do, and really not too difficult.
Languages that allow for more formal assumption-checking (especially before runtime) are even better, but comments have an additional benefit of being understood by a human directly.
I wish languages with static analysis could somehow allow authors to encode human-friendly/sematic errors that you often see as runtime exceptions into the static analysis itself...
I consider it a big risk of errors.
When some code is changed, will all related comments be rewritten too? I doubt it.
And then you end up with a codebase which indicate A but comments which clearly spell out B, and you as a maintainer have no idea what to believe.
DRY. Don’t repeat yourself. The comments should not double up for the code. That’s just future maintenance nightmare.
If every function is just a few lines long, the comments are easier to keep synchronized, and if a function drops out of service, it should eventually be garbage collected with its now-irrelevant comments.
Kind of the whole problem is when there are weird corner cases going on that straddle function boundaries.
I'm not saying that's a good thing; mind you - but nor is it always trivially avoidable, especially if code needs to be concurrency and/or exception safe -- or in general whenever the statements your function consists of have surprising and opaque behavior based on system state, particularly if said state is hard to grasp due to being implicit or dynamic, or simply large and complex.
It gets a little tiresome threading the relevant state to every function that needs it, but it’s worthwhile in the end.
The fundamental issue remains that sometimes your knowledge about that state (whether the classical kind or a proper parameter) can be complex and dependent on what happened elsewhere, especially if the codebase your in was grown into that situation, and not designed like that per-se. A comprehensible set of preconditions and postconditions isn't always a luxury you have, certainly not at first.
If the problem has "hub and spokes" topology, i.e. it's relevant to multiple places in code that all reference a single location, put a comment describing the issue in that single location, and everywhere else put a comment with a reference. //Warning. See comment in [that location].
If there's no single best place for the detailed comment, put it in some design notes file, and put a comment with a reference to that file in all the affected places.
DRY can, and should be, applied to comments as well.
I'm a bigger fan of WET(Write Everything Twice). Usually the first iteration of a component you don't understand enough of the domain space to get the abstractions right. So use that first attempt to explore the issues/problems/corner cases. Once well understood, rewrite it into something concise and well abstracted.
I've also find that if you try to re-write a third time you'll end up being to clever in trying to predict where a system will evolve and get you right back into the same situation as the first iteration.
Everything in moderation, including moderation itself.
Can you name a few examples where you encountered this? In my career (30 years programming) I've never seen it. I believe it's a common, poor excuse for not writing enough comments.
The benefits of comments are well-understood. For me personally they often helped compensate sloppy code, (non-obvious) assumptions and prevented bad solutions because I reconsidered while writing (embarrassing) comments.
when you have to maintain a large codebase modified thousands of times in 15+ years, every single comment is invaluable.
//here we go baby!
//do not touch!
//1 ... //2 ... //3 .. //8
//This happens only for CompletedOrders (while in reality it was the opposite)
//This calculates the notional in the same currency (while it was converting it in GBP)
//This is extremely important, never delete (and after that there was a bunch of commented code)
Honestly I don’t remember all of them, I tend to use my memory for more important things, but I bet that if you had seen only 1/10 of the bad comments that I have encountered you would have a different opinion. Btw in your post you are explaining exactly why I hate comments:
“For me personally they often helped compensate sloppy code” - the solution is obviously to fix the sloppy code, not to write a comment because you are too lazy to fix it
“(non-obvious) assumptions” - this is probably the only legitimate reason to write a comment.
“and prevented bad solutions because I reconsidered while writing (embarrassing) comments.” - this has nothing to do with the comments given that at the end you didn’t write it. Thinking more about the code instead of trying to comment it gives you the same result, if not even better.
> “(non-obvious) assumptions” - this is probably the only legitimate reason to write a comment.
If that's what you think, I rest my case...
As someone who is clearly a fan of comments, are you saying you’ve never modified a comment to make it more accurate to what the code is doing?
Tests can help understand the interface, but they don't help to understand the rationale behind it, the underlying abstraction, or implementation caveats.
One time, I inherited a big steaming pile of spaghetti my co-worker wrote to meet a tight deadline, before being shifted to another project. That code implemented one of the key functions of the application, and half a year later, the customer demanded extensive changes. Believe me, I would have paid half my monthly salary the just to have a third of the comments that we see in this Kubernetes file.
--
[0] - http://antitrust.slated.org/www.iowaconsumercase.org/011607/...
And a bit more on the same from clean code: http://www.kyleblaney.com/software-blog/2012/6/29/comments-a...
obviously
More lines doesn't mean more complex, it's the same logic just the logic is named now and more reusable. It's possibly not the best example, as the logic is minimal, but when the logic becomes more complex, wrapping it and naming it becomes very powerful. We're creatures of abstraction.
> You've made the code more generic for what you currently think future changes are going to look like, which may or may not be accurate.
I'd argue that I've reduced the number of reasons the code has to change, which should be a goal while programming. If we change how we calculate a leap day, we don't touch how we modify a leap day, which means we're less likely to cause adverse side effects.
> And in the process you've split dateIsOnLeapDay and convertLeapDayToPreviousDay into separate functions, so if someone is tracking down a bug in line 10, they need to jump to lines 20 and 21 to figure out that the associated code is in line 15
They should be separate functions, they are separate things.
> In a large program these would get even further separated over time
Is it really a problem if they are separated? What links them? There could be plenty of reasons for wanting to call one without the other.
Maybe this is just different instincts/experience and I'm not saying you're wrong, but my feeling here is that you do actually want to change them at the same time. Suppose we decide instead of adding a day to February 29, we keep the months the same and add a festival day at the end of every fourth year, numbered 13/1. Then modifying the festival day to 13/0 is wrong - the day before 13/1 is now 12/31.
If you have one function for "fix leap days for reporting purposes" then you're fine, and you've set the abstraction in a good place (or at least good for my example case, I will totally concede there are other examples!). When you edit the is-it-the-leap-day line of code, you'll see the subtract-one line of code directly below, and if you forget, your reviewers are likely to notice. And you haven't really made things noticeably worse for the case where the customer says "Actually we need February 29 rounded to March 1, instead", it's not distracting to have that line of code above where you are (and if anything it's useful to have that comment, so that if this is a different customer asking you realize that you need to not break expectations for your first customer).
I am something of a skeptic of reusing very short pieces of code - for instance, my team's own codebase has a poorly-designed function for calling a subprocess and swallowing certain types of errors from a very specific command, and in a code review I had to tell someone to just use subprocess.check_output(), which does the same thing but without the modified behavior which they probably didn't want. Abstraction makes sense when there is a meaningful concept to abstract. (Similarly, I am also very much not a fan of getters and setters; I think most people are better off with a structure with public fields, because I have very rarely seen it be useful to convert a trivial getter/setter to a non-trivial one without bothering to look at how callers use it, and it is useful to rename the field and see which code fails to compile / no longer passes tests now.)
transactionsLeapDayAdjusted = transactions.map(t => t.date == '29-Feb' ? {date: '28-Feb', ...t} : t)
I think around 10% of your code as comments is a good measure, but also remember that you may not revisit a module for years, and you will come to appreciate each and every breadcrumb you left which leads back to your original state of mind when you wrote it. If you measure code quality as maintainability, then comments can indeed increase code quality, just non-linearly.
I think people should spend more time commenting/documenting across the board. I'd much rather have verbose commenting that is unhelpful that I can skip, versus minimal commenting and code that is overly optimized and hard to parse. The less thinking I have to do to pick up where the last person left off, the better.
I would say that yes, in general high comment:code ratio tends to be higher quality.
Comments become useful when behavior is implicitly tricky. Ideally you'd make the "trickiness" tangible and expressible in-whatever-language you're in, but that's not always easy to do.
Just the opposite. It typically indicates a history of rigorously documenting terrible code. Sometimes, comments come from complex business requirements or other external constraint. Documenting the former is largely an anti-pattern while documenting the later is hugely useful.
I find that comments can be a last ditch effort to make hacky convoluted code look better than it is. It can be an indication of lack of thought and planning and later obsessive documentation to make up for it.
That's always been my instinct.
Elsewhere in this thread Ousterhout's book is mentioned; I like his advice about always placing comments in the most obvious places and as close to the code they affect as possible. This way, you can't miss them, and and it's hard to forget to update them.
Woah, that sounds like a pretty dumb feature. Auto-collapsing whole functions is useful, but auto-collapsing docstrings sounds like a recipe for disaster. People write docstrings and inline comments for a reason.
Comments don't need to be near the code they affect. They don't even need to be in the same file - consider if you've organized your code by features, but the comment relates to a layer than spans multiple features.
Eventually I tracked down this line:
// writes to the log file at c:\...\xyz.log
AppendToLog(message);
Eventually I looked inside the AppendToLog method. It was writing to another file in a completely different path :)
That was when I stopped bothering to read comments. They always lie, and I couldn't even blame the programmer who changed the AppendToLog method -- the comment wasn't inside the method, it was on a call. I can't honestly expect someone who changes a method to look for all the places where that method is called and make sure any existing comments match the change.
This file isn't that heavily commented. Do you look at many OSS projects for comparison? Though when things get complicated with many branches and function reentries it makes me wonder whether the problem would have been better solved with declarative logic that handles the procedural mess for you. (It might also be much higher quality since you may unlock access to various formal methods and go beyond unit tests. Though perhaps for example there's a vetted TLA+ spec not shown that this controller is based on.)
I don't think doc'ing every function is unusual, usefully doing so is less common though. Comments in the function body also aren't that rare, though it might indicate a place for better factoring e.g. just more function calls on descriptive/suggestive names. (Having more functions will help in not having to stub out (and deeply stub) so much in a behavioral test, too, since you can get away with just mocking the function call instead of the potentially hairy state logic the function does underneath.)
I see an example at a random spot for a couple improvements in naming (in my ignorant opinion, I don't know about kubernetes) -- though the fact I feel able to express even a weak opinion on an improvement suggests the comments were reasonable. I've seen code less hairy but with no comments or useful tests and without a need to really understand it I just want to move along pretending I saw nothing.
Look at the set of ifs at L591. The first if is a null check with part of the explanation on L592, better to remove that part and have a function call, something like "claimWasDeleted(claim)". The matching else if on 615 checks for an empty string name, I'm not sure but I think its explanation is at L634 and the empty string check could be "isClaimPending(claim)", and maybe move the mode mismatch check to its own else if before the isClaimPending block and give it a better name. I appreciate the comment on L635 telling me why the next line of code on 641 is done (it may likely not be clear from the commit history, which can be another place for whys) though with the isClaimPending change the comment and code might be replaced with a fn call with the details in the fn doc. I'm also reminded of an idea in more expressive languages to annotate purely optimization metadata of any kind (inlining being the simplest) and being able to toggle it on/off for extra QA in a test suite. Anyway the next elif on 643 and its comment, could be something like "isVolumeBoundToClaimProperly(claim, volume)". You get the idea.
Comments when done correctly are vital.
Correctly done comments are a one in a million thing. In my experience, they are utterly surpassed by "i = i + 1; // increment i" style comments. (Seriously, I'm working on code written by someone who teaches programming and he writes this type of comment.)
The code to do this is simple, but not concise, leaving a comment so I can scan the function and skip 5-10 lines doing something trivial is nice.
Nope, imho code with lots of comments is generally crappy code. It's littered with comments to explain the sloppy code they couldn't make clear because they're bad programmers. Good programmers use few comments, write simple clear code that doesn't require explaining, and leave comments about why something was done rather than simply trying to explain what the code does.
Code never lies, comments often do; don't trust comments that explain the code.
To me, comments are noise, and code is signal; the code is what actually executes.
It's one thing to have a summary of intent at the start of a listing, that should not count towards the code:comments ratio.
Once the code begins however, there should be a minimum of comments necessary - especially in a high-level language not constrained to assembly-level instructions.
In assembly listings it was common to have two columns, the code on the left and comments which often resembled high-level pseudo code on the right. Here's some representative apollo guidance computer source:
MAKEPRIO CAF ZERO
TS COPINDEX
TC LINUSCHR
TCF HIPRIO # LINUS RETURN
CA FLAGWRD4
MASK OCT20100 # IS PRIO IN ENDIDLE OR BUSY
CCS A
TCF PRIOBORT # YES, ABORT
https://github.com/chrislgarry/Apollo-11/blob/master/Comanch...
You did, to be fair, say that comments at the top shouldn't count. But I think that depends a lot on personal (and language) style towards multiple files and multiple units within a file - for instance, none of this code is object-oriented, and comments above each class make sense in an object-oriented language. I think as the language gets more concise - Go is a lot more concise than the Apollo assembly language - you're going to need to have the same amount of prose to explain what you're doing but a lot fewer lines of code to get anything done, and it makes sense to have comments above each function or each block, because that's really the comparable unit.
Comments are noise to the compiler, but code is both a communication between humans and from humans to machines. To imply that only what executes is signal and all else noise is to ignore half the purpose of code, which is documentation.
And despite what a lot of people want to believe, code itself is often not sufficiently self-documenting.
If you don't want to spend your career rewriting the same thing over and over again for slightly different business use cases and platforms, comments are incredibly valuable. (On the other hand, I guess there's a lot of job security in being hired to write the same thing many times....)