4 ms·

Here's an example of a very useful comment: # using heapsort instead of quicksort # because we care about the worst case heapsort(data) Code alone cann

Here's an example of a very useful comment: # using heapsort instead of quicksort # because we care about the worst case heapsort(data) Code alone cannot tell the reader why heapsort is chosen, nor whether that's how the code should be. And tests are unlikely to capture this either.

vinay_ys 4y ago
While it is useful to have a short comment like this in the code as a quick reminder, such design choices belong in a more detailed technical design document which explores multiple design options and provides rationale for making the final choice. Usually, there will be a trove of useful stuff in code review comments etc that should find its way into such a technical design document. To ensure it gets done alongside code, I prefer wiki docs that sit alongside code and doc changes can be part of the same code commit as well.
- xmprt 4y ago
  If comments can lie then design documents can lie even more. There are three tiers of documentation in my mind. Code, comments, and documents. Code can't lie (although it can be misleading with improper naming or bad abstractions). Comments can be completely false (although they're easier to catch because the comments sit alongside the code) Design docs almost always lies and it's hard to catch the lies because a design doc written 2 months ago is never going to match the resulting code perfectly. In my experience, people rarely update documentation unless there's a dedicated employee/developer advocate who is helping enforce and drive the culture.
  - trashtester 4y ago
    > a design doc written 2 months ago is never going to match the resulting code perfectly. That depends how you're writing design docs. If a design doc contains arbitrary flowchars about what happens in the code, it will change. But if you come up with designs similar to design patterns, you can document the pattern once, and use it in your code the way you use a design pattern. Concepts that span a large codebase is also enormously paintful to figure out from reading the code. For instance object models / microservice architectures, complex state machines or security frameworks are hard to catch implicitly from reading the code.
  - vincnetas 4y ago
    You got it backwards. You have design documents and they should be source of truth. If you change your code and code now contradicts design docs, it's the code that is wrong. Said that design docs should not be super detailed, they should capture general assumptions and conventions, code can have all the details, but code cant contradict design docs. But when you too lazy to update design docs, code and docs get out of sync. And it's always easier to just leave docs unchanged, because "hey, code works". It's a bit similar to word processor compatibility issues on linux and windows. You can have perfect implementation of specification on linux, but Microsoft does some changes that are not specification compliant and now a doc created on windows does not open on linux. And from users perspective it's linux fault that file can't be opened, even though it's actually microsoft that is not following specs.
    - quickthrower2 4y ago
      Your last paragraph is pretty much why in practice the design specs go out of sync. The reward systems reward correct code not correct docs. Unless the team is very strict and code reviews doc changes.
- drekipus 4y ago
  > such design choices belong in a more detailed technical design document which explores multiple design options and provides rationale for making the final choice. We're talking about a line of code that has 2 words here. "A method to sort something" and "something to sort".
- DoingIsLearning 4y ago
  > such design choices belong in a more detailed technical design document I agree with you but in my experience unless you are writing this documentation in a regulated industry (with external incentives to keep the documentation up to date) the technical docs will eventually go stale or out-of-sync with the actual implementation.
- AlotOfReading 4y ago
  In my experience, source code tends to survive forever. Things outside it don't, whether through attrition via document retention periods, the IT department noticing nobody uses that document archive and shutting it down to cut costs (not before being told "nobody uses that!"), or hundreds of other possible causes. I've worked on code older than I am and those little comments that probably should have been design docs were a godsend. Nothing else had survived.
trashtester 4y ago
I agree, these are cases when you need comments inlined in the code. Describing what a single code is doing is only useful, if the comment is easier to read than the code itself. That can be useful for arcane cases, such as dirty regular expressions, bash scripts with several layers of pipes, or non-standard pointer manipulation in C. Much more important, is the why, as in your example. But there is also the what when it applies to code design that spans more than a few lines of code. This requires documentation. As long as we follow a design pattern, the documentation is covered by the pattern description. But if we invent a novel design, either a one-off or a new design pattern, it needs to be described in text and figures, not just inline in the code. Similarly, and even to some extent when staying inside a design pattern, object models need a conseptual design, where the ideas and purposes behind each class or interface is explained, as well as where the relationships between classes are visualised.
- hinkley 4y ago
  Commit messages and calling people out for breaking commit message history works to an extent. If you can keep someone from breaking commit history badly enough. I still maintain we're awaiting another generation of version control where commentary on code is cumulative, instead of just set at the time of commit.
  - joshka 4y ago
    Yeah, it's hard to retrospectively unbreak commit history on legacy projects. Git notes[1] might be a possible solution, but I've not had that be particularly useful in practice (due to lack of support in various other systems). [1]: https://git-scm.com/docs/git-notes https://git-scm.com/docs/git-notes
snovv_crash 4y ago
How about: import heapsort as worstcase_nlogn_sort worstcase_nlogn_sort(data)
mostlylurks 4y ago
It is excessive to claim that code cannot express the "why". You can almost always express the "why" with code if you really want to, though whether or not that's a good idea is another matter. For example, you could express the same information that the comment in your example provides in the following manner with self-documenting code. enum PerformancePrioritization { FastWorstCase, FastAverageCase } function sort(data: List<TypeOfData>, performancePrioritization: PerformancePrioritization) { if (performancePrioritization == FastWorstCase) { heapsort(data); } else { quicksort(data); } } ... sort(data, FastWorstCase);
- mannykannot 4y ago
  I feel that this code is its own counter-example.
- PhineasRex 4y ago
  An extra nine lines and extra runtime complexity just to avoid putting it in a comment.
  - mostlylurks 4y ago
    I did explicitly mention that it might not be a good idea, though this is an exaggerated case since situations like these realistically constitute a very small portion of actual codebases. But a sibling comment [0] did provide an example that would give a much simpler approach to accomplishing the same thing in this particular instance, with zero extra lines and runtime complexity. [0]: https://news.ycombinator.com/item?id=31822657 https://news.ycombinator.com/item?id=31822657
- renox 4y ago
  And then you have an automated checker which tells you FastAverageCase isn't used..
- spc476 4y ago
  Okay, here's some code I wrote [1], with the existing comment: /*------------------------------------------------------------------------ ; For the RR OPT, the class field is actually the size of the UDP payload, ; and the TTL field are a bunch of flags. We have a separate field for ; the UDP payload size, so here we make the adjustment behind the scenes ; so you don't have to know this crap. ;-------------------------------------------------------------------------*/ if (answer->generic.type == RR_OPT) { answer->opt.class = answer->opt.udp_payload; answer->opt.ttl = ((data->rcode >> 4) & 0xFF) << 24 | (answer->opt.version & 0xFF) << 16 | (answer->opt.fdo ? 0x80 : 0x00) << 8 | htons(answer->opt.z & 0xFFFF) ; } I'd like to see how I could do away with the comment and make this more "self-documenting". [1] Code to encode/decode DNS packets: https://github.com/spc476/SPCDNS/blob/05aead581acf050edf610c6cb5cf86d33c686062/src/codec.c#L864 https://github.com/spc476/SPCDNS/blob/05aead581acf050edf610c...

vinay_ys 4y ago

While it is useful to have a short comment like this in the code as a quick reminder, such design choices belong in a more detailed technical design document which explores multiple design options and provides rationale for making the final choice. Usually, there will be a trove of useful stuff in code review comments etc that should find its way into such a technical design document. To ensure it gets done alongside code, I prefer wiki docs that sit alongside code and doc changes can be part of the same code commit as well.

xmprt 4y ago
If comments can lie then design documents can lie even more. There are three tiers of documentation in my mind. Code, comments, and documents. Code can't lie (although it can be misleading with improper naming or bad abstractions). Comments can be completely false (although they're easier to catch because the comments sit alongside the code) Design docs almost always lies and it's hard to catch the lies because a design doc written 2 months ago is never going to match the resulting code perfectly. In my experience, people rarely update documentation unless there's a dedicated employee/developer advocate who is helping enforce and drive the culture.
- trashtester 4y ago
  > a design doc written 2 months ago is never going to match the resulting code perfectly. That depends how you're writing design docs. If a design doc contains arbitrary flowchars about what happens in the code, it will change. But if you come up with designs similar to design patterns, you can document the pattern once, and use it in your code the way you use a design pattern. Concepts that span a large codebase is also enormously paintful to figure out from reading the code. For instance object models / microservice architectures, complex state machines or security frameworks are hard to catch implicitly from reading the code.
- vincnetas 4y ago
  You got it backwards. You have design documents and they should be source of truth. If you change your code and code now contradicts design docs, it's the code that is wrong. Said that design docs should not be super detailed, they should capture general assumptions and conventions, code can have all the details, but code cant contradict design docs. But when you too lazy to update design docs, code and docs get out of sync. And it's always easier to just leave docs unchanged, because "hey, code works". It's a bit similar to word processor compatibility issues on linux and windows. You can have perfect implementation of specification on linux, but Microsoft does some changes that are not specification compliant and now a doc created on windows does not open on linux. And from users perspective it's linux fault that file can't be opened, even though it's actually microsoft that is not following specs.
  - quickthrower2 4y ago
    Your last paragraph is pretty much why in practice the design specs go out of sync. The reward systems reward correct code not correct docs. Unless the team is very strict and code reviews doc changes.
drekipus 4y ago
> such design choices belong in a more detailed technical design document which explores multiple design options and provides rationale for making the final choice. We're talking about a line of code that has 2 words here. "A method to sort something" and "something to sort".
DoingIsLearning 4y ago
> such design choices belong in a more detailed technical design document I agree with you but in my experience unless you are writing this documentation in a regulated industry (with external incentives to keep the documentation up to date) the technical docs will eventually go stale or out-of-sync with the actual implementation.
AlotOfReading 4y ago
In my experience, source code tends to survive forever. Things outside it don't, whether through attrition via document retention periods, the IT department noticing nobody uses that document archive and shutting it down to cut costs (not before being told "nobody uses that!"), or hundreds of other possible causes. I've worked on code older than I am and those little comments that probably should have been design docs were a godsend. Nothing else had survived.

trashtester 4y ago

I agree, these are cases when you need comments inlined in the code. Describing what a single code is doing is only useful, if the comment is easier to read than the code itself. That can be useful for arcane cases, such as dirty regular expressions, bash scripts with several layers of pipes, or non-standard pointer manipulation in C. Much more important, is the why, as in your example. But there is also the what when it applies to code design that spans more than a few lines of code. This requires documentation. As long as we follow a design pattern, the documentation is covered by the pattern description. But if we invent a novel design, either a one-off or a new design pattern, it needs to be described in text and figures, not just inline in the code. Similarly, and even to some extent when staying inside a design pattern, object models need a conseptual design, where the ideas and purposes behind each class or interface is explained, as well as where the relationships between classes are visualised.

hinkley 4y ago
Commit messages and calling people out for breaking commit message history works to an extent. If you can keep someone from breaking commit history badly enough. I still maintain we're awaiting another generation of version control where commentary on code is cumulative, instead of just set at the time of commit.
- joshka 4y ago
  Yeah, it's hard to retrospectively unbreak commit history on legacy projects. Git notes[1] might be a possible solution, but I've not had that be particularly useful in practice (due to lack of support in various other systems). [1]: https://git-scm.com/docs/git-notes https://git-scm.com/docs/git-notes

snovv_crash 4y ago

How about: import heapsort as worstcase_nlogn_sort worstcase_nlogn_sort(data)

mostlylurks 4y ago

It is excessive to claim that code cannot express the "why". You can almost always express the "why" with code if you really want to, though whether or not that's a good idea is another matter. For example, you could express the same information that the comment in your example provides in the following manner with self-documenting code. enum PerformancePrioritization { FastWorstCase, FastAverageCase } function sort(data: List<TypeOfData>, performancePrioritization: PerformancePrioritization) { if (performancePrioritization == FastWorstCase) { heapsort(data); } else { quicksort(data); } } ... sort(data, FastWorstCase);

mannykannot 4y ago
I feel that this code is its own counter-example.
PhineasRex 4y ago
An extra nine lines and extra runtime complexity just to avoid putting it in a comment.
- mostlylurks 4y ago
  I did explicitly mention that it might not be a good idea, though this is an exaggerated case since situations like these realistically constitute a very small portion of actual codebases. But a sibling comment [0] did provide an example that would give a much simpler approach to accomplishing the same thing in this particular instance, with zero extra lines and runtime complexity. [0]: https://news.ycombinator.com/item?id=31822657 https://news.ycombinator.com/item?id=31822657
renox 4y ago
And then you have an automated checker which tells you FastAverageCase isn't used..
spc476 4y ago
Okay, here's some code I wrote [1], with the existing comment: /*------------------------------------------------------------------------ ; For the RR OPT, the class field is actually the size of the UDP payload, ; and the TTL field are a bunch of flags. We have a separate field for ; the UDP payload size, so here we make the adjustment behind the scenes ; so you don't have to know this crap. ;-------------------------------------------------------------------------*/ if (answer->generic.type == RR_OPT) { answer->opt.class = answer->opt.udp_payload; answer->opt.ttl = ((data->rcode >> 4) & 0xFF) << 24 | (answer->opt.version & 0xFF) << 16 | (answer->opt.fdo ? 0x80 : 0x00) << 8 | htons(answer->opt.z & 0xFFFF) ; } I'd like to see how I could do away with the comment and make this more "self-documenting". [1] Code to encode/decode DNS packets: https://github.com/spc476/SPCDNS/blob/05aead581acf050edf610c6cb5cf86d33c686062/src/codec.c#L864 https://github.com/spc476/SPCDNS/blob/05aead581acf050edf610c...