Useful and useless code comments(blog.jim-nielsen.com) |
Useful and useless code comments(blog.jim-nielsen.com) |
I have gone back and fourth on comments over the years, but currently I'm working on a solo project which has been in serious development for some time, and using comments in this way is really useful.
Especially because it's a domain I haven't done a ton of work in before, I've found my workflow is often to create a few source files, type out in comments what the component should do, and bit by bit hone in on how that should translate into code.
It's also great for placeholders: if I'm not ready to fill in an implementation yet, I can just type out a lengthy description in the function body, and when I come back a week later I can remember what the intent was.
/* https://medium.com/@roxeteer/javascript-one-liner-to-get-elements-text-content-without-its-child-nodes-8e59269d1e71 */
return await this.page.$eval(selector, e => {
const s = [].reduce.call(e.childNodes, function(a, b) { return a + (b.nodeType === 3 ? b.textContent : ''); }, '');
return s.replace(/\s+/g,' ').trim();
}); return this.page.$eval(selector, (node) => Array
.from(node.childNodes)
.filter((child) => child.nodeType == Node.TEXT_NODE)
.map((child) => child.nodeValue)
.join('')
.replace(/\s+/g, ' ')
.trim()
); // Add a vertical scroll bar
vScrollBar = new JScrollBar(JScrollBar.VERTICAL);
add(vScrollBar, BorderLayout.EAST);
But there actually are useless comments. Like when people are told to explain every class member and one ends up with
DatabaseConnection conn; // connection to the database
This really is just clutter and the next problem is that it needs to be updated alongside the code. I once had a colleague advocating adding a comment for every parameter that a method has and then he would himself not update these if parameters were removed. That really is not very good.
1) If you know the author of the code (includes yourself) and you know their comments are a good indication of the code, you just read them and move along. No need to understand what they do.
2) If you don't know the author or you know their comments are often out of date (which you can check with git blame but let's ignore that) the comments tells you what the original intention was, and with an already mental model its easier to read code and understand what it's doing, even if it contains mistakes (in the worst case the code will have nothing to do, so it will be like if you had no comment).
When it comes to comments, be verbose!
I would do
// We allow scrolling in two directions
hScrollBar = new JScrollBar(scrollBar, HORIZONTAL);
add(hScrollBar, BorderLayout.SOUTH);
vScrollBar = new JScrollBar(JScrollBar.VERTICAL);
add(vScrollBar, BorderLayout.EAST);
caretX = 0;
caretY = 0;
caretMemX = null;
However, in my experience, it's common for either:
1. Code to get changed, but the relevant comments are unchanged, or
2. Comments that are just as hard to understand as the code
In both cases, it's even harder to understand the comment+code than if there was no comment at all
Also: they function like an anchor tag. If you are spelunking a new code base for a specific functional area, a search in plain English would let you jump to the right spot more quickly.
To make matters worse, it's hard to do math with these things, thus net benefit can't possibly be calculated.
If you're writing code for a tutorial for newbies then yeah, it makes sense to add obvious comments. But that's really it. The author is overstating his case.
Most development has been, is and will be for the foreseeable future outsourced, not in house. I don’t like that, but I have seen things.
Too much comment about is never a severe problem. At most a distraction.
99% of code that matters is written by experienced people in my opinion. I.e. the OS, the browser, your favourite apps, websites and programming libraries.
Would you agree with that?
If comments are de-emphasized (e.g. grayed out compared to code) you will get bad comments and comments out of sync with the code.
I think 2 IDE features would make things easier for everyone:
1) have an option to remove code comments from view if you don't like them
2) tie a code comment to a piece of code and enforce it. If you change that piece of code, you should be forced to update and tied comments, or force to check that the comment is still correct. And have the fact that you approve a comment should be a part of the check in, so people can trace it.
2) IMHO, it's better to assign a premium for the comment fix at GitHub. Your time is expensive, so it's better to outsource this distracting task to someone in India, because English is the official language in India. And track all that via a smart contract on a blockchain.
// Add a vertical scroll bar
vScrollBar = new JScrollBar(JScrollBar.VERTICAL);
add(vScrollBar, BorderLayout.EAST);
- It’s creating a visual boundary that aids to the readability of the code. It adds context within a large set of instructions.
- It’s providing context that might not be there for the person who is reading the code. Just because you think the name of that function and what you’re passing into it clearly describes the instruction, doesn’t mean that everyone else does. At least not in a snap. The comment helps to reduce the cognitive load of reading the code because it’s explaining the instruction in plain english.
- The comment itself could be part of a larger narrative that is trying to explain what that file is doing. It’s not there to make the obvious more obvious. It’s there to make the whole file obvious which is evidently important to write readable code.
Look. I know there are purists that get offended with obvious code comments. But you cannot expect everyone to have the same proficiency you do. Writing good code, also means writing code that a newbie can at least comprehend. And sometimes that means explaining in plain english what could be already obvious to you.
People like to think they are writing code like prose that is delightful to the reader, but many times are just writing reader hostile code.
Write code focusing on clarity. Not elegance by obscurity. Make it clear, add structure, add visual boundaries. Ask yourself if what you think is obvious is obvious to everyone. If in doubt explain it with a comment. I’ll rather see an obvious comment and say “well that’s obvious” than spending valuable time trying to understand what certain piece of code is doing.
I wouldn't add a heading above every paragraph, but I would might above every few paragraphs. In code, this translates to every 5-15 lines of code. Here's some code I wrote recently that shows this (https://github.com/canonical/pebble/blob/b152ff448bbe7d08c39...):
func writeFile(item writeFilesItem, source io.Reader) error {
if !pathpkg.IsAbs(item.Path) {
return nonAbsolutePathError(item.Path)
}
// Create parent directory if needed
if item.MakeDirs {
err := os.MkdirAll(pathpkg.Dir(item.Path), 0o755)
if err != nil {
return fmt.Errorf("cannot create directory: %w", err)
}
}
// Atomically write file content to destination.
perm, err := parsePermissions(item.Permissions, 0o644)
if err != nil {
return err
}
uid, gid, err := normalizeUidGid(item.UserID, item.GroupID, item.User, item.Group)
if err != nil {
return fmt.Errorf("cannot look up user and group: %w", err)
}
sysUid, sysGid := sys.UserID(osutil.NoChown), sys.GroupID(osutil.NoChown)
if uid != nil && gid != nil {
sysUid, sysGid = sys.UserID(*uid), sys.GroupID(*gid)
}
return atomicWriteChown(item.Path, source, perm, 0, sysUid, sysGid)
}I especially appreciate how under syntax highlighting the comments provide a visually offset natural language outline or summary of the code.
If you're writing example code or documentation, by all means, add this kind of comment.
Otherwise, if someone can't comprehend these particular two lines without reading the comment, they need to get back to the drawing board. You can't have noisy code like that in production at the off-chance that somebody with zero competence needs to read it. The expected case is that a competent developer needs to find "the thing", which means they need to be able to scan the code quickly without having to read everything twice. That's what you should optimize for.
The DRY principle also applies, this sort of error is not uncommon:
// Add a vertical scroll bar
hScrollBar = new JScrollBar(JScrollBar.HORIZONTAL);add_vertical_scrollbar()
- Unless the function is extremely clear like max() or min(), you will have to read the function and jump around the code. I feel people underestimate the cognitive overhead required for this. This gets worse the more arguments are required to pass around. Linear code is much easier to read.
- More lines of code, a comment adds 1 line while a function or method adds at least 4. Also, code that has to be separated over multiple functions is practically always longer than linear code. Doing this to the extreme can have quite an effect.
- Naming issues, it can be hard to think of a good name for a function and it almost never describes it perfectly. Misnaming things leads to bad assumptions and leaky abstractions. Comments have the luxury of potentially conveying more information and are also less important to get right.
If you use a lot of vertical scrollbars then by all means go ahead and abstract it, but I'd generally pick the comment.
When reading the calling function, add_vertical_scrollbar() makes perfect sense. But when you read the code and you encounter add_vertical_scrollbar(), it is not at all clear where it fits in the grand scheme of things. You could of course add a comment "Meant to be called from this-or-that-function", but that kinda defeats the purpose.
We could make the function more general, which is not always as simple as in this case, but then we're doing more than just extracting some code. Or we could call the function add_vertical_scrollbar_east() instead, but that gets unwieldy pretty fast if there are more tunables.
Don't get me wrong, I do see value in extracting code into functions, and I often do exactly that. But in doing so I notice some drawbacks of that approach that IMO are not generally addressed by small-function-advocates.
It's very easy to generate code that "jumbles" tasks. You set "x" coordinate of your point in one place, then do some calculations for "y" coordinate, then set it, then do something unrelated, and then set the color of your point. Such code does not make it obvious that you could create a "set_point(y, y, color)" function.
When coding in paragraphs, you organize your code into chunks so that each one "does one thing and does it well". It's natural then to consider whether a particular chunk could be a separate function.
Often it is, but sometimes it is not. First, creating a separate function has costs: choosing an appropriate name, designing an interface, etc. If the name and/or interface are not well-designed, a separate function can decrease readability: consider a function called "fix" that has 10 positional arguments...
Second, especially if your chunk is within a loop or two, the code may be too tightly coupled with the state within your code. You would need additional and perhaps modifiable "state" parameters to your function, making the resulting code more complicated and less comprehensible than the original one.
And of course, if your hypothetical function cannot be inlined (perhaps because your language does not even have the facility, or for other reasons), you would pay the price of a function call, which may matter in a tight loop.
In general it's always good to consider factoring chunks of code as a separate function, but sometimes upon this consideration you should reject it.
Some use it as a method to separate code into independent, concise blocks that represent a singe step in a larger algorithm. Do one thing and do it well.
The other is code reuse, if a function is not reused in other places why create it at all? That's the reason for preferring comments rather than fragmenting code into add_vertical_scrollbar() functions. It's easier to read from top to bottom instead of jumping all over the place
This is literally always the right answer. Create a self-explanatory function or just use readable variable names.
Or even createScrollbar(Direction.VERTICAL) if you hate repeating yourself.
And push back hard against linter/prettifier rules that remove all developer discretion from layout and structure.
That said, I've started to develop strong views on some eslint rules. Usually around things being errors that should be warnings in development, like logging messages or debugger or unused variables. I like to run in CI with max warnings being zero to catch them then.
Consider the scenario that a second developer has to change your code in the future, for example the decision is made that a vertical scrollbar isn't required anymore and a horizontal one is required instead. It is very possible you end up a comment that completely contradicts what the code is actually doing.
// Add a vertical scroll bar
hScrollBar = new JScrollBar(JScrollBar.HORIZTONAL);
add(hScrollBar, BorderLayout.EAST);
> given they don't have to change them to meet the requirements
What are the requirements? Code review is standard practice, so I see clear code as part of the requirements.
As they say, "Documentation is like sex: when it is good, it is very, very good; and when it is bad, it is better than nothing" :-)
Yeah, it could trick me, but this is no different to any other types of sloppiness.
Should we stop giving meaningful names to variables and functions, because some developer can change what a function does, or variable represents, but fail to update the names to reflect the fact?
Most of the time when I have to look at the code with out of date comments, after paying some initial "tax" of confusion and lost time, it becomes clear that the comment is out of date(git helps with this, too). So I take time and fix the comment. Or in rare cases when I still don't understand what the comment should say, I put a "FIXME: out of date comment" above the offending line.
One thing I give you: when reading code, I find comments explaining why the code does X, more useful than comments stating the the code does X...
For me, another giant benefit is that I can scan through the function and understand what it does just by reading the comment. The energy saved by those comments can be spent on writing more code :)
Today I ran into this piece of code:
// POST
xhr.open("POST", "...");
// Content-Type
xhr.setRequestHeader("Content-Type", "...");
// SOAPAction
xhr.setRequestHeader("SOAPAction", "...");It is, if the alternative is to just refactor your code to extract a method so that it reads like:
addVerticalScrollBar();
The main problem with your suggestion is that you're wasting precious space to say the same thing twice.
More importantly, your comments adds nothing important or relevant to those reading the code, such as why did you felt the need to add a vertical scrollbars to begin with. That's the sort of thing that's relevant to those reading the code.
As long as the surrounding function is not overly complex, I don't see the advantage of having to jump back and forth from mini function to mini function to understand what the code is doing.
I agree with this so much that I have added (some time ago) a syntax for that to Next Generation Shell.
https://ilya-sher.org/2019/10/21/section-syntax-next-generat...
TL;DR:
section "Add a vertical scroll bar" {
...
}- Compared to most user facing development, where probably >75% of code is purely for interaction (and potentially more repetitive, obvious, and readable), a LOT of embedded code is not so readable, and quite often context is key. Good comments (even on apparently obvious code) almost always carry some context (why do this here, and why in this order, why the specific delay before reading the ADC, etc)
- Because almost all the code is in some way business logic, it is definitely easier at times to read the flow of comments, rather than work out directly from the code which register someone is trying to set, and why, or why some seemingly arbitrary value is being incremented (that is actually being read by a parallel task somewhere else)
- As always, good comments are as much for yourself six months from now, as for the guy replacing you 6 years from now. Sometimes, it is just a good way to document your thought process, as well as provide reasonably up to date system documentation, since ACTUAL design documentation almost always gets neglected.
Basically - comment as much as you can, and as much as you need. Try and keep it up to date. It doesn't matter if it's seemingly obvious, or discussing the reasoning behind a particular architectural decision. Nobody I know has ever complained about too many comments, and if anything, more would have always been better.
Most of the developers, which hate comments, hate them because they cannot write proper comments, i.e. they are hiding their own weakness, to look smarter than they are.
Finding the place in the code where X is being done would be perfectly feasible by consciously reading the code, but I want to save that energy. Letting my visual cortex pattern-match on a syntax-highlighted comment that looks a certain way is much cheaper and faster and can be done while scrolling through it without stopping. If you can make that comment so it's more useful beyond that, so much the better.
Tricks like that come into play once you're past the language proficiency / commenting policy hurdles and feel the impact of your visual code design on your personal attention and energy budget.
I find code with lots of comments and "fluff" to be a drag to read and navigate and generally work with. I would rather have less to scroll to begin with, than have comments help me jump to a specific spot.
For example, I just replaced a well documented pair of functions that compute a checksum. The original implementation was about thirty lines, including comments and empty lines. My implementation uses a denser style and is just a single function with three lines in the body. I also removed a function argument to make the interface both simpler to use and also less general. KISS & YAGNI: if the more general version is ever needed, then it is trivial to add and make the simple version a wrapper for it. Until a real need shows up, I don't want it.
Of course my changes go way beyond comments, but this kind of thing can make an order-of-magnitude difference in code size (as measured in lines). I would rather work with 300 lines than 3000 lines, and the reduced need for jumping/scrolling around lessens the importance of navigation aids. Also, I find that the aids that are there anyways (function names, etc.) become more useful when there's no fluff to render them ineffective.
> // Add a horizontal scroll bar
> hScrollBar = new JScrollBar(scrollBar, HORIZONTAL);
> add(hScrollBar, BorderLayout.SOUTH);
> addHorizontalScrollBar()What these comments do is give the code structure, and giving an outsider a better/faster view into the code, when he has no internal model for the library/framework/API/language which helps him recognize the statements (this outsider might be the author himself, or "future me").
Not knowing what these lines do, I can at first glance (i.e. skimming through the code) understand the author's intentions, what each line is responsible for. This is simply reduction of mental load and I even do this just for myself when trying to wrap my head around a new library or language.
On the other hand, if you are writing for a (large) internal codebase, where you need to find a tradeoff between reduction of mental load and bloat, such comments can quickly become a nuisance...
Then again, the cost of useless comment is usually much smaller than the cost of mental load (everyone knows how to ignore comments, unless they fill pages...)
All this being said, I can only recommend "The Art of Readable Code" - Boswell [1].
[1] https://www.oreilly.com/library/view/the-art-of/978144931848...
The never, ever happens consistently.
So soon you have the "man with two watches" problem. The code says one thing, and the comments say another. You could fix it, and sometimes you spend that time, but inevitably you also stop reading the comments, since they're not reliable.
I've never seen this not happen in comment heavy projects.
> So soon you have the "man with two watches" problem. The code says one thing, and the comments say another. You could fix it, and sometimes you spend that time, but inevitably you also stop reading the comments, since they're not reliable.
When you think about it, function and variable names are comments. Should we not use those because they need to be maintained with the code?
Obvious comments are great for code reviews because it's a validation that what you intended to do is what your code actually does.
From the article, the comment about adding a vertical scroll bar allows me to review the code and to check that it does indeed add a vertical scroll bar and not something else.
Did the developer intend to require password lengths of at least 8 characters? A simple comment would tell me what the developer wanted to do and then I can verify that it was done correctly.
Eg. I like paragraphed comments, because you know what the next 5 lines will do and you can quickly skip to the part you need.
It's not always possible to have a method with only 5 lines. There are use-cases where you want a big method doing a lot of things together, because it's complex.
Eg. For a government project i needed to connect to the internal network for executing certain requests and all that logic was in one HttpProxyClass, shared between the api + service/deamon
It's great that somebody is reviewing every single line of code and probably will catch it when folks modify the code but inevitably forget to update the comment, and it becomes out of sync - becomes misleading.
If the reviewing is not vigilant that's where you end up: you don't know what to trust, the comment or the code. And the code wins, so that _what_ and _how_ type of comment becomes a net negative.
I understand that my situation will not reflect the average developer. But it is good to see both points of view.
Over multiple courses including software engineering, comment quality and or correct use of comments was never in the syllabus.
But in several courses they wanted to see the code heavily commented, to the point where you were just re-writing the code as a comment and getting a green tick instead of a red "more comments!" note. I legitimately believe that my educators just didn't know any better themselves.
These days I live by the mantra: Comment to explain WHY not HOW, since the WHY is lost to time whereas the HOW is often self-descriptive.
PS - [0]Still my favorite comment of all time. I'm still pissed that someone tried to remove it from the article on Wikipedia (in particular as the comment is almost as famous as the code).
[0] https://en.wikipedia.org/wiki/Fast_inverse_square_root#Overv...
I very distinctly recall being told in an introductory programming course:
"Code is written for people, not computers."
That really stuck with me. What that lecturer meant was that first and foremost code must be written to be comprehensible to other people. The particular techniques used, whether commenting, intelligent variable names, extracting sections into functions or using abstractions are just a means to that end.
And layers of abstraction or indirection can easily harm comprehension rather than help. It all comes down to the circumstances. Do whatever is necessary to make it as comprehensible to other people as possible; everything else is secondary.
It's actually a somewhat decent way to catch cheaters. As it provides a better likeness score between two code comparisons.
Obviously, I'm not talking about comment like this (which I see all the time):
// adds 1 to x
x += 1;
I generally think you shouldn't comment to explain your use of a language feature, even an unusual one. The exception: when there exists an idiomatic alternative which you intentionally didn't use, then a comment can explain why.
I worked in the Firefox codebase for a while, and what I would have given for a simple comment that describes intentions or results; where instead I had to fire up searchfox.org and make a deep dive into some (equally sparsely commented) functions.
There is some notion here, that these comments help new programmers, but I think as soon as your codebase is sufficiently big, there will be always parts you won't be familiar with, and these comments certainly help you scan over them quickly
Also, I'm curious to see how your experience was working on the firefox code base. I haven't checked out the code base myself, but have built from source a few times. Curious to see how the Rust progress has been going on the code base and which parts of firefox are currently implemented in Rust.
I think you see Rust when there are really big projects that can be treated as a library. For instance the network team is writing their QUIC implementation in Rust – called neqo – and I can imagine that this was possible because it's low level enough and separate enough from the rest of Firefox.
It's of course also a resourcing question. For instance the download-manager code in Firefox is a big, big mess and it's not really owned by anyone. But it works, is safe, and handles a lot edge cases. Rewriting this thing (even again in Javascript) would be an enormous effort, and that's true for so many parts of the code base that might benefit from rewrites. A rewrite would be nice, but there probably will always be more important things to work on.
That's because I write my code's doc string first (or JavaDoc or whatever), then write the body in English comments or pseudocode on rare occasion, then write the code beneath and between the conments that are my English plan.
I will also write additional comments about anything unintuitive, complex or unusual, even slightly, because I know that the code will be read many more times in the upcoming decades. (I have code I wrote in 1994 in production at one company, and another hitting its 20th anniversary this year still in production. Do you even remember libg++? Me neither, but the code still uses it...)
My favorite comments are actually "what" comments, but that clarify something opaque about the code, e.g.
const R = 6371e3;
const φ1 = lat1 * Math.PI/180;
const φ2 = lat2 * Math.PI/180;
const Δφ = (lat2-lat1) * Math.PI/180;
const Δλ = (lon2-lon1) * Math.PI/180;
const a = Math.sin(Δφ/2) * Math.sin(Δφ/2) +
Math.cos(φ1) * Math.cos(φ2) *
Math.sin(Δλ/2) * Math.sin(Δλ/2);
const c = 2 * Math.atan2(Math.sqrt(a), Math.sqrt(1-a));
const d = R * c;
// haversine distance formula, result is d in meters
// Vincenty algorithm too slow for our use case
// and we don't need that level of accuracy.
I still mostly abide by that, but I've had an opportunity to write some larger programs for the first time in a few years (a hazard of teaching programming is you find yourself working with pretty small programs), and noticed that the comments started become a part of my conversation with myself. I'd drop in quick reminders for what I intended to do in a function before going moving on to something else. When I came back and then wrote the code corresponding to the comments: well now it's redundant, but it wasn't. I stopped cleaning that stuff up (I'd clean it if I had to present or publish it, of course). The comments kept some context of my thinking when I wrote it, what order I did things in, etc. It would be gibberish for another, but it wasn't for me, especially after a few days, or even longer. (Note: I'm a pretty fast touch-typist, so muttering to myself doesn't invoke much of a productivity cost. I also note that commit messages are another great place to capture that sort of context for yourself, and not using that opportunity is a sin.)
So, I've changed some of my greybeard aphorisms around commenting: "When commenting code, consider the needs of the person who will be maintaining this code. Who will probably be you. Be nice to future you."
It only becomes a problem when comments are used to paper over the author's own indifference to making the code itself legible, and that's where I expect critiques like this truly come from. Comments without legible code do not help, as the comments are not a proof of correctness or verification that the author's intent has been carried out. You still need to read the code to understand the program.
Code legibility is not unlike legibility in written language. Short sentences and paragraphs, clear transitions, well-defined hierarchy, consistent structure. Say more with less. Comments are often like footnotes: they elaborate upon the author's intent, provide a mental palette cleanser, elaborate on a digression in a way that limits distraction, or occasionally break the fourth wall.
Comments are improper when they try to serve other means. For instance comments about invalid uses of a function/class/etc. when the type system can be used to prevent illegal usages of code. Or when comments are used to explain what is happening in extremely branchy, anti-modular, or otherwise illegible code.
It is also my experience that writing comments (even the so-called flower boxes, e.g., Javadoc/JSDoc) force me to think about what the code is doing and its readability to future maintainers. If I find myself having to over-explain the block, I might rewrite it or extract it out to a function or class, the name of which summarizes the intent, therefore eliminating the need for the comment.
I have a way that I like to do it. Other people on my team have their own ways. My way is not hurting them. Their way is not hurting me. Either of us trying to impose our way on the other would hurt both of us.
The code says how. The commit says what, and where the edited lines were/are in which files, when the edits were committed, and who committed them.
The comments should say why, iff a why is needed.
IMO this is exactly what unit tests should be used for. Replace these comments with unit tests and you're doing TDD. The practical difference is by stating your intention as unit tests, you not only verify your initial implementation is correct but also that any future changes remain correct.
Do test-driven development, but consider deleting these internal tests once you have scoped out how your initial implementation should work.
The problem is more that you might not have a comment when you need one. There's a lot of times when the author has thought a lot about something and thus understands it, but someone glancing over the code might not.
Example:
def caterpillarMethod(A, s):
n = len(A)
front, total = 0, 0
for back in xrange(n):
while (front < n and total + A[front] <= s):
total += A[front]
front += 1
if total == s:
return True
total -= A[back]
# =============================================================================
# /\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/
# =============================================================================
I find myself writing such separator comment blocks in userscripts quite a bit; and stylesheets where I do not have any tooling attached to it (e.g. mediawiki css).
Comments should be reserved for non-obvious information. Github link, complex mathematics etc. Further, at what point does the relevance of a comment end? This ambiguity doesn't exist with a function.
function addHorizontalScrollbar() {
hScrollBar = new JScrollBar(scrollBar, HORIZONTAL);
add(hScrollBar, BorderLayout.SOUTH);
}
function addVerticalScrollbar() {
vScrollBar = new JScrollBar(JScrollBar.VERTICAL);
add(vScrollBar, BorderLayout.EAST);
}
function initializeScrollbarCaretPositions() {
caretX = 0;
caretY = 0;
caretMemX = null;
} // Add a horizontal scroll bar
hScrollBar = new JScrollBar(scrollBar, HORIZONTAL);
add(hScrollBar, BorderLayout.SOUTH);
void addHorizontalScrollBar() {
hScrollBar = new JScrollBar(scrollBar, HORIZONTAL);
add(hScrollBar, BorderLayout.SOUTH);
}
...
addHorizontalScrollBar();
(edit: disclaimer -- Snark aside, I should say that I personally tend toward the latter option. But I am absolutely not prepared to try and convince anyone that it is, in any objective sense, better. It's just an aesthetic preference. Maybe even a nervous habit.)
I would argue that that this more readable as for what is going on:
addHorizontalScrollbar() ;
addVerticalScrollbar();
addHeader();
addDayColums();
addFooter();
Some of this depends on whether I'm using an IDE or not. In an IDE I can just Ctrl click myself through. Then again I doubt anyone these days is using _just_ vi to code something complex enough to want multiple files. I used vi configured as an IDE way back but I do admit that's been like 15 years. Dunno what that looks like today. Nowadays I'm a JetBrains user, mainly in Java and Java/TypeScript and very little Python.
add(new JScrollBar(scrollBar, HORIZONTAL), BorderLayout.SOUTH);
A lifetime ago when I wrote assembly code I would write a comment like this to explain every subsequent 5-10 lines of opaque incantation. Higher level (especially functional) languages tend to have fewer issues like this.
The first example might need a comment, but everything I need to know is right there in one place and not distributed across the codebase.
// Build the data table view
...
// Add horizontal scroll bar to the data table view
// This is needed if the table has more than N columns
tableHScroll = // etc
// Build the data navigation sidebar
...
With methods extracted you could do the exact same but on actual code and help your future self that forgot all about this class or some other poor developer that has never seen this UI or the code before.
With the function I think, "is it and if it is what else does it change?". Is that function definition actually doing what it says it is? So now I have to add another stack frame to my mental model, go to definition on the function, confirm that is indeed what it claims to do, pop the frame and resume my reading of the parent method.
With the comment I get the statement of intent and can see if the code matches inline. Now it's fair to say a good code review culture (and immutability) might create code where there's more room to trust named functions, but I've not seen it yet.
Whether I read a function name or comment personally that does pre occupy my mind and sets the mental frame for what comes next. If the comment or function name say doX() or //this does X then I expect it to do that but need to be on the lookout for whether it actually does that.
With a function though I can easily step out if this part turns out not to be interesting, I can easily step over it next round if debugging, I don't need to keep track of where //this does X ends etc.
Of course none of these help if the previous author was really bad at what they were trying to communicate. They can both implement doY() in a function called doX() and have //do Y code run in with //do X for example by interleaving statements for both. If we assume we'll intentioned authors for both though, I would tend to see more pros for structuring code with functions than via comments.
Including - and I had some resistance to this - breaking out a variable or function solely to give something a name that needs describing.
This way you have one source of truth. It can be wrong, but you only update it once, and it never conflicts with itself.
I understand what you mean by "function and variable names are comments". We do use them to describe what the code does. Still, they are actually code :)
That’s more useful than dealing with stale documentation which can also be outdated. At least with code comments you have contextual proximity.
I even find "man with two watches"useful. When I see it, I _know_ that something is wrong here, and it's worth checking. Most of the time the comment is found to be "buggy" (and gets updated), but I have found quite a few real bugs in the code this way.
That being said most code out there is legacy of course...
Even after many years of programming in Perl I found comments like
# FROM hex, not TO hex!
useful.
This is particularly problematic for this sort of issue because it's impossible for a static analysis tool to pick up.
Something like that?
Update: I noticed you updated your comment.
So my reaction to your addition is that a class can always be abstracted to higher level. It will probably do to much at this point.
The logic was much simpler to scan/change if you comprehend everything it does. Instead of only partially by abstraction. The use case that one thing was called for one-use case wasn't applicable.
I like clean code and apply it a lot. But it's not applicable for everything. I think you just didn't encounter the more advanced cases yet where it isn't, since they are not that frequent.
The spirit of clean code is to make readable code. Not the reality is that you have to adjust to the situation. They're is code were comments make sense. Comments can give context and intent ( look at code of sqlite for example).
That said, I agree with both OP and GP. I personally prefer clarity take precedence over whatever code-commenting or code-formatting guidelines one might have in place. It is all about common sense, I guess.
A better analogy I think would be legal contracts with no headers and no numbered/bullet points. Headers in bold text have no value in a court, why not simply have a few pages of plain text that is self-explanatory? Also applies to comments to laws and religious texts.
I personally prefer clarity should take precedence over
Many programming interfaces are designed with no clarity in mind, sadly. Also many processes are not so straightforward and contain intermixed logic, like (0) get a raw request (1.a) get this, (2.a) get that, (3) log raw event with half-assed data for monitoring or debugging, ((1,2).b) check both and throw, (4) combine into a task, (5) log task, (6) proceed further. This creates zigzags in the code flow that you can’t reduce by abstracting things away, because it would blow up a complexity to insane levels.
I agree that on one hand micro commenting is dumb, but if your blocks are 3-4-line long it’s usually a sign of good structure, which is unrelated to commenting itself. A good structure is self-explanatory, but its flow may be complex (ask Brooks). Human attention-switching helpers seem useful to me, because they help seeing the flow. A short comment on what’s going on is like an inlined IIFE without accompanying visual/scope clutter.
Why not? The above could have easily contained a `ensureDirectoryExists(item)`. It would have the benefit of being more reusable, too.
// add vertical scroll bar
addHorizontalScrollBar(...)
I _know_ something is wrong.
Did the comment get out of synch with the code? Or the developer copied-and-pasted the wrong function? Or the function itself is misnamed? I don't know! Let's investigate... and fix whatever happens to be erroneous!
But when I see just
addHorizontalScrollBar(...)
all I know is that a horizontal bar seems to be added (provided the function name reflects its... er... function). I don't know whether it was intended behaviour. If it has not been, nothing indicates this at all.
I've actually caught real bugs this way, multiple times.
The problem with this is that it isn't actually solution. A name can't contain much that's helpful to know about a thing, which should be obvious given how much prose and narrative everyone produces and consumes. The program itself can only tell you what. Names can sometimes hint at intent, but they they're not very good at it (given the incentive for terseness). Comments are good for explaining the "whys" for a thing as well as intent. Those are important things to know when debugging a problem or trying to enhance something.
Comments shouldn't be thought of as the one thing you need to look at to understand some code. You really have to look at a whole constellation of things (comments, implementation, commit history, tribal knowledge) and synthesize them. When you take that approach, even out of date comments can be more useful than no comments.
> I understand what you mean by "function and variable names are comments". We do use them to describe what the code does. Still, they are actually code :)
They're not any more code than comments are. Run an obfuscator and change all names to random strings and everything will still work fine, just like if you ran a program to strip out comments.
If you refactor a function or variable to do something different, often it becomes obvious at the calling/usage site that something isn't quite right any more and the name gets fixed. Just having more instances of it gives you more opportunities to realise you need to fix the name.
It's absolutely not foolproof, you can obviously still have them get out of sync. I do find it's rarer, though, personally.
Also it imposes more burden on the author, and possibly could lead to developing the wrong abstraction, which we all know is bad.
One of those seems easier to me. Potentially our brains just work way differently but I have a much easier time remembering that function name I will step over.
I'm quite a visual thinker so I use the shape of the surrounding code to anchor my mental model of the code. If I have to jump to another file or function then I lose the shape and it disrupts my model. This is why on balance I (now) prefer longer functions over code split up.
It is never clear cut. But if you never reuse the same code, or it is targeting the wrong abstraction layer (you can't reuse the function without introducing another set of arguments) and you are merely encapsulating two already very readable lines of code, you might end up with a too much redirection and an increase in mental load.
What I'm trying to say is: It might be even harder for the outsider to follow/read the code, just simpler on first glance.
The code example of the OP is excellent because there are literally so many more avenues for code readability that are all superior to comments. First of all, the same code can literally be reused in the next line:
addScrollbar(HORIZONTAL);
addScrollbar(VERTICAL);
addScrollbars();
I said "taken to the extreme". Put differently: If you are going to wrap every pair of lines into separate functions as a way to structure your code, you code will end up becoming functions calling functions calling functions - and whenever you try to read the code, you are required to jump through all these redirections.
Where do you draw the line?
In short, I don't think adding depth to a hierarchy of function composition is always an answer. Flatter hierarchies can be easier to understand too.
addHorizontalScrollbar($style);
addVerticalScrollbar($style);
addHeader();
addDayColums($style);
addFooter($style);So...object methods?
What you seem to suggest is refactor the original code (which presumably was a single function to set everything up) so that variables used and the substeps are in one module (your new object), and the code that uses all of this is in another module. I am not convinced this really follows "high cohesion, low coupling" dogma, since you put together things that might be less related (different style variables) into the same module, and created a function, which almost all it does is calling into another module, and is highly dependent on it.
The problem ultimately is data relationships in the code can be a general graph, and trying to put a general graph into a hierarchical structure (much less one that actually has to follow the structure of operations on the data) always has to break cycles in some way. And how much you can really do it depends on cyclomatic number of the graph, and if it's high, no strategy is going to be good. AFAICT most of the OOP strategies of "dealing" with this problem are just creating more nodes in the graph, making the structure bigger, but not really reducing the cyclomatic number.
There isn't really a proper place to put such function.
In this example, you would like to define add_vertical_scrollbar() right where it is used as this is the proper context where it makes sense to read it. But if you define it where it's used it no longer adds any clarity.
A function makes the higher-level flow more clear and uncluttered than without either the function or comment, a comment may make it more clear but also makes it more cluttered.
> Unless the function is extremely clear like max() or min(), you will have to read the function and jump around the code. I feel people underestimate the cognitive overhead required for this. This gets worse the more arguments are required to pass around. Linear code is much easier to read.
Yes, function names should be extremely clear. But less code is easier to read than more code, so unless the contents (rather than the purpose) of the block of code is likely to be important every time you read the containing bit of code, you make the containing bit of code more readable by extracting the block.
> more lines of code, a comment adds 1 line while a function or method adds at least 4.
Depends on the language. In JS, for instance, a function definition can be a single line, so definition + call adds a minimum of 1 line, just like a comment. But a comment adds 1 line to the parent block of code, abstracting out a function at worst simplifies a single line, and usually reduces the lines of code in the parent block. So where the parent but not the content of the abstracted bit is important, it reduces code read, whereas the comment always increases it.
> Naming issues, it can be hard to think of a good name for a function and it almost never describes it perfectly.
Whether you can think of a clear name for the function is, IMO, part of the test for whether it is a logically coherent unit of work to abstract out to a function in the first place.
> Comments have the luxury of potentially conveying more information and are also less important to get right.
Comments are a pure addition of clutter to the code they are attached to; if they aren’t gotten right they are not just visual noise but misleading visual noise. Comments are no less important to get right than naming of functions.
It is subjective and depends on a given piece of code. In my experience functions (abstraction in general) often obfuscate code, because you cannot really see what they do without jumping to another place and breaking a flow of reading.
There are of course cases where factoring a piece of code into a function would make in more readable, but I cannot say this about any group of commented lines. And a function requires a comment too.
Already discussed in the post you are responding to: “unless the contents (rather than the purpose) of the block of code is likely to be important every time you read the containing bit of code, you make the containing bit of code more readable by extracting the block.”
> There are of course cases where factoring a piece of code into a function would make in more readable, but I cannot say this about any group of commented lines.
Also already addressed, e.g.: “Whether you can think of a clear name for the function is, IMO, part of the test for whether it is a logically coherent unit of work to abstract out to a function in the first place.”
> And a function requires a comment too.
Not necessarily. A function in the public interface of a module probably needs a doc comment or docstring, but we’re discussinf abstracting out a piece of functionality used once, so presumably this is to a private function in the same module, not part of the public interface of the module.
The call stack becomes very organized and readable when debugging.
Then the user has the choice to call earthDistance to get the program's preferred estimate of distance, or haversineDistance if they want to be certain about the implementation. Later, perhaps you add a new distance formula and earthDistance decides which method to use as appropriate.
Feels like the difference between inlining quicksort vs. calling quicksort() vs. calling sort(). In some situations you really are on a level where you need to know you are calling mergesort() and not quicksort() but some are fine with sort(). I doubt there are many examples of code I'd argue for inlining.
I mention this because I think it's great that the commenting style works with both our workflows and doesn't really imply rigid adherence to either one of them. I suspect we would find it comfortable to maintain each others' code despite the divergence in approach.
Myself I like coding in paragraphs very much, and giving the paragraphs one-liner headings (comments) definitely makes code for me more readable and easier to navigate.
I usually don't give a heading to a "paragraph" which is just one line, unless that line does something subtle. (Conway provides a definition of "subtle": if you need to think more than 10 seconds or consult the fine manual to figure out what the line does).
[1] https://www.oreilly.com/library/view/perl-best-practices/059...
https://www.perl.com/pub/2005/07/14/bestpractices.html/#7-co...
There are other recommendations on that page and in Conway's highly opinionated book which I disagree with (it's notorious for recommending Class::Std), but that point has stuck with me.
Note that Conway's example even includes three single-code-line paragraphs, all of which are commented. Perhaps Osterhout would not approve, or perhaps he would — each of those lines includes some subtlety, and each of the comments describes the "why" not the "what".
FWIW I don't always add a comment above single line paragraphs if the comment adds nothing. But neither does Conway comment "unpack arguments" above the first line in his example, nor say anything about the return statement — which to my mind illustrates that there's some flexibility to be expected in how you apply this technique.
Thanks for the link. I may have to find a copy of that book at the library :)
Unfortunately every place I've worked at sort of accidentally pits their programmers against each other in a sort of "who can get it done first" competition, with those who do a quick-and-dirty job getting credit, and those who clean up their proverbial workbench getting none. We've had gentlemen's agreements to not do this, and have even been able to collectively oust (i.e. get fired) a few serial violators, but there's just SUCH pressure to ship quickly that code review is basically a fictional thing like the flying spaghetti monster.
It's telling that we've got a MLOC codebase, and basically not a single piece of documentation - and every single place I've worked, in almost 2 decades, has been exactly like this.
Enterprise software is a wild ride. :|
I really hope these companies compensated you well...
I find it strange that people want to throw the baby out with the bathwater when it comes to commenting code just cause they've been bit before.
Generally if I see something happen in every job I've had throughout my career I start to doubt that there is a simple process solution.
We found a problem! Let start a startup: "Intelligent code review tool for smart developers, which want to keep comments in their code up to date". The tool will use AI to learn which parts of code are modified often in unison, and then propose to check them also during code review. It will also check for copy-pasted code blocks, and for snippets from StackOverflow with known bugs. The tool will reduce code review time by 10%, reduce number of bugs slipped trough code review by 20%, improve time to ship by 7%, bla bla bla, and instruct the coffee machine to prepare a coffee just in time for the review.
On the other hand, I have been bit plenty of times by incorrect documentation outside of code about libraries and APIs.
File-level and function-header type comments that tend to generate docs (eg. Doxygen) are certainly more likely to fall out of date (easy to miss in a PR diff etc) and I don't really support using them anyway.