5 ms·

I’m on the same page. Just look at the given example. Does it look wasteful to say that you’re adding a vertical bar when you have the following line of code?:

I’m on the same page. Just look at the given example. Does it look wasteful to say that you’re adding a vertical bar when you have the following line of code?: // Add a vertical scroll bar vScrollBar = new JScrollBar(JScrollBar.VERTICAL); add(vScrollBar, BorderLayout.EAST); Perhaps. But the comment is making a lot more than simply describing what’s below of it. The comment is doing the following things: - 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.

nitrogen 5y ago
Make it clear, add structure, add visual boundaries. And push back hard against linter/prettifier rules that remove all developer discretion from layout and structure.
- wilsonrocks 5y ago
  I'm all in on prettifying... If it's done automatically. I find it such a good tradeoff to not have to think about it that I'm happy to accept the times when I think my discretion would do it better. 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.
benhoyt 5y ago
Yeah, I like the "visual boundaries" way of framing it. A line of whitespace is a visual boundary, of course, but I find the // comment above it acts as a heading. Like a bold heading above a couple of paragraphs of text in a document. 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/b152ff448bbe7d08c39f56871b643599949d5da8/internal/daemon/api_files.go#L456-L483 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) }
- rectang 5y ago
  I know this style by the name "coding in commented paragraphs". I learned about it from Damian Conway early in my career, and the idea resonated with me so strongly that I immediately adopted it and have used it every since. I especially appreciate how under syntax highlighting the comments provide a visually offset natural language outline or summary of the code.
  - NicoJuicy 5y ago
    Aha, i did this. Knowing a name for it makes it official to defend on PR's :p
  - tomgp 5y ago
rualca 5y ago
> Does it look wasteful to say that you’re adding a vertical bar when you have the following line of code? It is, if the alternative is to just refactor your code to extract a method so that it reads like: addVerticalScrollBar(); No comment, cleaner code, smaller functions/methods, everything is easier to read and follow and reason about. 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.
- rerx 5y ago
  Then these two lines of code move away from the context of the surrounding function. There may also be something specific about how the scrollbar is added in this instance, requiring a very complicated name if you want to convey that. 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.
manojlds 5y ago
If that line of code is not obvious, why not add a function and do add_vertical_scrollbar()
- kwinten 5y ago
  Careful, you might trigger about half of HN with your proposal of readable code and small functions. 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.
  - bipson 5y ago
    It triggers half of HN because taken at extreme (abusing) this advice will render code unreadable again. 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.
    - kwinten 5y ago
      If the lines of code were already very readable, they wouldn't require a comment. Comments are for interfaces or complex or inobvious code that can't easily be made more obvious simply through an expression of code IMO. 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); Do you frequently or always need to add both scrollbars to the UI? Another chance for reducing repetition and error-proneness when refactoring: addScrollbars(); There's no increase in mental load because chances are you'll never need to navigate into these functions to know what they are doing at all, you can read over them as you would normally when you read the "This adds a horizontal scrollbar" comment, and trust that the implementation does what it says on the tin.
morbia 5y ago
The problem with this approach comes down to when the functionality of the code changes. 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); Which suddenly ends up being far worse than having no comment at all. You could say that the second developer should be diligent and ensure they fix these, but given they don't have to change them to meet the requirements you are inviting the possibility that it could happen.
- tomerv 5y ago
  I never understood this line of reasoning. If you're changing a line of code you should at least look at the whole function that contains it, and update the comments according to the change. > 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.
  - morbia 5y ago
    Developers and code reviewers make mistakes. If a change involves looking at code numerous trivial comments, the probability that both the developer and the reviewers misses at least one increases. This is particularly problematic for this sort of issue because it's impossible for a static analysis tool to pick up.
- D13Fd 5y ago
  The second developer updates or deletes the comment. Yes, this is a maintenance burden. The question is whether the burden outweighs the benefit.
- IlliOnato 5y ago
  I disagree that it "ends being far worse than having no comments at all". I think it's still helpful: 1) still provides the visual boundary for the following chunk of code 2) still tells me that the code has something to do with scroll bars 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...
mt40 5y ago
I do this all the time as well ever since I knew about this technique. 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 :)
b93rn 5y ago
Yes, this is bad. If it was not clear enough that this created a VScrollBar, then create a function CreateVerticalScrollBar(). Why? Well this does exactly the same as commenting the code, except that this can easily be renamed with your IDE if the code base changes. Comments will always be outdated, functions not.
incrudible 5y ago
> 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. 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);
- drran 5y ago
  Books need no covers, because if a reader cannot comprehend a book title from the book content, then (s)he must go back to the school. /s
  - ignoramous 5y ago
    False equivalence. 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.
    - wruza 5y ago
      If a sense is discussed by camps with opposing opinions, it is far from common by definition. 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.
ptx 5y ago
I tend to agree that sections headings are OK (although it might be better to extract the sections to separate functions) but some people take it further than that. Today I ran into this piece of code: // POST xhr.open("POST", "..."); // Content-Type xhr.setRequestHeader("Content-Type", "..."); // SOAPAction xhr.setRequestHeader("SOAPAction", "...");
ilyash 5y ago
> visual boundary 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-generation-shell/ https://ilya-sher.org/2019/10/21/section-syntax-next-generat... TL;DR: section "Add a vertical scroll bar" { ... }

nitrogen 5y ago

Make it clear, add structure, add visual boundaries. And push back hard against linter/prettifier rules that remove all developer discretion from layout and structure.

wilsonrocks 5y ago
I'm all in on prettifying... If it's done automatically. I find it such a good tradeoff to not have to think about it that I'm happy to accept the times when I think my discretion would do it better. 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.

benhoyt 5y ago

Yeah, I like the "visual boundaries" way of framing it. A line of whitespace is a visual boundary, of course, but I find the // comment above it acts as a heading. Like a bold heading above a couple of paragraphs of text in a document. 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/b152ff448bbe7d08c39f56871b643599949d5da8/internal/daemon/api_files.go#L456-L483 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) }

rectang 5y ago
I know this style by the name "coding in commented paragraphs". I learned about it from Damian Conway early in my career, and the idea resonated with me so strongly that I immediately adopted it and have used it every since. I especially appreciate how under syntax highlighting the comments provide a visually offset natural language outline or summary of the code.
- NicoJuicy 5y ago
  Aha, i did this. Knowing a name for it makes it official to defend on PR's :p
- tomgp 5y ago

rualca 5y ago

> Does it look wasteful to say that you’re adding a vertical bar when you have the following line of code? It is, if the alternative is to just refactor your code to extract a method so that it reads like: addVerticalScrollBar(); No comment, cleaner code, smaller functions/methods, everything is easier to read and follow and reason about. 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.

rerx 5y ago
Then these two lines of code move away from the context of the surrounding function. There may also be something specific about how the scrollbar is added in this instance, requiring a very complicated name if you want to convey that. 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.

manojlds 5y ago

If that line of code is not obvious, why not add a function and do add_vertical_scrollbar()

kwinten 5y ago
Careful, you might trigger about half of HN with your proposal of readable code and small functions. 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.
- bipson 5y ago
  It triggers half of HN because taken at extreme (abusing) this advice will render code unreadable again. 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.
  - kwinten 5y ago
    If the lines of code were already very readable, they wouldn't require a comment. Comments are for interfaces or complex or inobvious code that can't easily be made more obvious simply through an expression of code IMO. 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); Do you frequently or always need to add both scrollbars to the UI? Another chance for reducing repetition and error-proneness when refactoring: addScrollbars(); There's no increase in mental load because chances are you'll never need to navigate into these functions to know what they are doing at all, you can read over them as you would normally when you read the "This adds a horizontal scrollbar" comment, and trust that the implementation does what it says on the tin.

morbia 5y ago

The problem with this approach comes down to when the functionality of the code changes. 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); Which suddenly ends up being far worse than having no comment at all. You could say that the second developer should be diligent and ensure they fix these, but given they don't have to change them to meet the requirements you are inviting the possibility that it could happen.

tomerv 5y ago
I never understood this line of reasoning. If you're changing a line of code you should at least look at the whole function that contains it, and update the comments according to the change. > 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.
- morbia 5y ago
  Developers and code reviewers make mistakes. If a change involves looking at code numerous trivial comments, the probability that both the developer and the reviewers misses at least one increases. This is particularly problematic for this sort of issue because it's impossible for a static analysis tool to pick up.
D13Fd 5y ago
The second developer updates or deletes the comment. Yes, this is a maintenance burden. The question is whether the burden outweighs the benefit.
IlliOnato 5y ago
I disagree that it "ends being far worse than having no comments at all". I think it's still helpful: 1) still provides the visual boundary for the following chunk of code 2) still tells me that the code has something to do with scroll bars 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...

mt40 5y ago

I do this all the time as well ever since I knew about this technique. 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 :)

b93rn 5y ago

Yes, this is bad. If it was not clear enough that this created a VScrollBar, then create a function CreateVerticalScrollBar(). Why? Well this does exactly the same as commenting the code, except that this can easily be renamed with your IDE if the code base changes. Comments will always be outdated, functions not.

incrudible 5y ago

> 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. 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);

drran 5y ago
Books need no covers, because if a reader cannot comprehend a book title from the book content, then (s)he must go back to the school. /s
- ignoramous 5y ago
  False equivalence. 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.
  - wruza 5y ago
    If a sense is discussed by camps with opposing opinions, it is far from common by definition. 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.

ptx 5y ago

I tend to agree that sections headings are OK (although it might be better to extract the sections to separate functions) but some people take it further than that. Today I ran into this piece of code: // POST xhr.open("POST", "..."); // Content-Type xhr.setRequestHeader("Content-Type", "..."); // SOAPAction xhr.setRequestHeader("SOAPAction", "...");

ilyash 5y ago

> visual boundary 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-generation-shell/ https://ilya-sher.org/2019/10/21/section-syntax-next-generat... TL;DR: section "Add a vertical scroll bar" { ... }