Beware the Siren Song of Comments • Hacker News Board

Re: Beware the Siren Song of Comments

#11

Post by kazinator » Wed, Aug 27, 2014, 5:59 PM UTC

Comments which reveal the intent of a block of code are very useful. They provide useful clues precisely when the code does not meet the expressed intent:

    /* Dear Maintainer,
     * I write the following in the sincere hope that
     * it sorts the variables a, b and c in ascending order.
     */

    if (a > b) swap(a, b);
    if (b > c) swap(b, c);

They are useful when the code is complicated and the intent is not obvious. Or when there can be multiple plausible intents based on what the requirements are. Sometimes code contains implementations of requirements that are not spelled out in any requirements document. A high level requirement can break down into lower-level ones in umpteen ways. Whatever isn't in the detailed design document is only in the comments and the code. A programmer's statement of intent can in fact be a requirement specification: the only such document for that piece of code.

Re: Beware the Siren Song of Comments

#12

Post by rubyn00bie » Wed, Aug 27, 2014, 6:01 PM UTC

I agree that patently obvious comments are not necessary... and descriptive naming helps avoid patently obvious documentation... But I disagree with adding more code to replace what would be a single line of documentation. To me that's insanity.

My teams find more and more value from documentation as they are forced to return to code months/years later.

And commit logs as the place to document performance issues? Seems like an easy way to make sure that a "weird" bit of code gets removed, and breaking your app.

As with all things, ideologies and blanket statements suck. I think what was talked about in this article could've been much better communicated if it wasn't so polarizing.

I've found comments are only not added or updated when people feel rushed. Or a culture which doesn't put value in them... Which is a problem with one's organization not the act of writing comments.

P.s. This is an edit: isn't it rather peculiar someone tasked with annotating all text doesn't annotate their code? Irony?

Re: Beware the Siren Song of Comments

#13

Post by chillingeffect » Wed, Aug 27, 2014, 6:02 PM UTC

timr wrote: post #10

"Comments decay. They aren’t compiled, and they’ll never get executed at runtime. If they become out of date or incorrect, no test is going to fail and no user is going to complain. Programmers work around them out of fear that “somebody might need this comment or it might provide some value in the future”, pushing them along far after they’re useful" The developers who say this are ticking time bombs of laziness on…

>"director of engineering",

...at Genius.

Re: Beware the Siren Song of Comments

#14

Post by wehadfun » Wed, Aug 27, 2014, 6:03 PM UTC

1st Point: use self documenting code instead of commenting | Ok fine

2nd Point: Explain performance hacks in commit description | might was well not explain them at all. This is a great place to hide the reason for the performance hack

3rd point: Don't use TODO / FIXME | I'm on the fence.

4th Point: Don't comment out old code use version control for old stuff | Ok

Re: Beware the Siren Song of Comments

#15

Post by chillingeffect » Wed, Aug 27, 2014, 6:06 PM UTC

I do agree about the clutter of auto-generated comments.

And to some extent some comments do decay. However, with experience, we learn to write comments with a minimal decay profile. As an example, avoid including constants in comments. Also, if you find yourself writing the same text over and over again, it's a sign you're being too redundant and can elevate that comment to a high level.

Re: Beware the Siren Song of Comments

#16

Post by arjie » Wed, Aug 27, 2014, 6:11 PM UTC

bunderbunder wrote: post #9

> The right place to document [performance] hacks is a commit message. This advice strikes me as patently insane. Serious performance hacks often involve subtle interactions among different parts of the code. When that happens, properly documenting them requires explaining what's going on in a line of code, for every line of code, right next to that line of code. Just sticking a wall of text somewhere else entirely a…

Likely to be using a workflow that uses `git blame` or equivalent to understand the context of code. The problem that is intended to fix is that comments don't stay in sync with the code they're commenting about. The problem it introduces is discoverability. You have to search to see where this code came from in order to understand it. That isn't going to happen unless you're disciplined. And we're all undisciplined at some point or the other.

Re: Beware the Siren Song of Comments

#17

Post by joseraul » Wed, Aug 27, 2014, 6:16 PM UTC

> Let’s say you want to resurrect this reslug_all_tag_pages method. Just use: $ git log -Sreslug_all_tag_pages

You "only" need to remember that the method is named reslug_all_tag_pages.

Re: Beware the Siren Song of Comments

#18

Post by Guvante » Wed, Aug 27, 2014, 6:22 PM UTC

timr wrote: post #10

"Comments decay. They aren’t compiled, and they’ll never get executed at runtime. If they become out of date or incorrect, no test is going to fail and no user is going to complain. Programmers work around them out of fear that “somebody might need this comment or it might provide some value in the future”, pushing them along far after they’re useful" The developers who say this are ticking time bombs of laziness on…

> Comments only "decay" if you don't read them, or if you don't take the 30 seconds to update them when you make a change

Comment decay is a rehash of "code that isn't written is never incorrect". Aka every change you make introduces an opportunity to make a mistake, including both writing and updating comments.

Re: Beware the Siren Song of Comments

#19

Post by gknoy » Wed, Aug 27, 2014, 6:30 PM UTC

Am I the only one that doesn't find looking up commit messages as a followup to `git blame` as convenient as a code comment?

When I see something like,

  42   def wobble(self):
  43       # Make sure the vent core has been frog-blasted, 
  44       # so that we know {rare bad things} won't happen
  45       frog_blast(self.vent_core)
  46       self.do_important_things()

I find that a much faster answer to "Do I really need to foo the bar?" than if I have to decode it:

  42   def wobble(self):
  43       frog_blast(self.vent_core)  # see commit msg
  44       self.do_important_things()

  git blame wobble.py
  git log abcd4242

  Author: Bob
  
    Ensure that vent core is properly blasted

    We rarely have frogs, but the vent core must be cleared.

Is this a matter of insufficient knowledge of my IDE? (I'd love to be able to easily show the last commit messages for specific lines in a tooltip or other popup.) Having to context-switch into commit-sleuth mode seems (to me) to be more disruptive than having the code comment.

Re: Beware the Siren Song of Comments

#20

Post by _delirium » Wed, Aug 27, 2014, 6:36 PM UTC

arjie wrote: post #16

bunderbunder wrote: post #9

> The right place to document [performance] hacks is a commit message. This advice strikes me as patently insane. Serious performance hacks often involve subtle interactions among different parts of the code. When that happens, properly documenting them requires explaining what's going on in a line of code, for every line of code, right next to that line of code. Just sticking a wall of text somewhere else entirely a…

Likely to be using a workflow that uses `git blame` or equivalent to understand the context of code. The problem that is intended to fix is that comments don't stay in sync with the code they're commenting about. The problem it introduces is discoverability. You have to search to see where this code came from in order to understand it. That isn't going to happen unless you're disciplined. And we're all undisciplined…

I only find blame-type tools useful for discovering either recent context, or context of rarely-touched code. They typically only easily show you the most recent change to a line of code, though you can sometimes dig through older changes in a kind of per-line commit history. So older changes get more and more buried under an accretion of more recent ones. To find the commit message discussing a performance hack from 8 years ago, you'd have to wade through all the more recent commits that also touched those lines of code (refactoring, fixing unrelated bugs, fixing library bitrot, etc.). I'd rather that important context stay right there in the code, not get buried among hundreds or thousands of other commits.