
Beware the Siren Song of Comments - wwarnerandrew
http://www.leastastonished.com/blog/2014/08/27/beware-the-siren-song-of-comments/
======
timr
_" 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 your team.

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. And if you're not doing that,
then _you 're not doing your job_, because the previous developer left the
comment for a reason, and your defeatist philosophy of "comments are useless"
is implicitly overriding that developer's judgment. Respect your colleagues
enough to at least read what they've written and update it when it's
incorrect!

It's pretty disheartening to hear this from a "director of engineering",
actually. I expect it from a new grad who has never worked on a team, or with
code that has to live for more than a quarterly project, but I want a team
leader to be doing _everything possible_ to improve communication amongst team
members. That includes compelling them to write and maintain good comments.

~~~
Guvante
> 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.

------
fortpoint
I've found quite the opposite. Most developers hate comments because they
represent extra work. That work manifests itself in the creation and upkeep of
the code. We've found that addressing comments as part of our code review
process made a big difference in keeping things up to date.

Regarding the issue of expressiveness: I never find myself commenting about
what is obvious in the code itself. Often I'm writing about some thinking
behind why something was done a certain way OR identifying expectations of
downstream systems that can't be expressed using code. Code enough in
environments where you're interacting with multiple disparate systems and
you'll find that some well placed comment providing the broader context for
the code is extremely helpful to folks encountering the code the 1st time or
to yourself a year later when you're incorporating a new feature and have
forgotten about all the concerns wrapped up in the code.

With the todo issue- we require folks to include a JIRA issue number along
with the todo. If something was left undone then it needs to be recognized
where we manage our backlog and flagged as a technical debt to be addressed.

~~~
davidgerard
+1. Commenting code _or config files_ with an issue number and a date turns
out to be a lifesaver. Your future self will thank his past self (instead of
cursing him as usual).

(I am a sysadmin, not a dev, but pretty much everything Operations writes
counts as code for these purposes.)

------
bunderbunder
> 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 and leaving it up to your successor to go find it and figure out how
the explanation cross-references with the code is _not_ an adequate
alternative.

~~~
arjie
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.

~~~
_delirium
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.

------
kazinator
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.

------
cyanoacry
This approach works well in only one situation: when your code isn't doing
anything substantial in terms of data processing.

For me, comments are best used at explaining _why this is happening_, not what
is happening.

By far one of the best examples of code commenting I've seen is the
explanation of the L2ARC (the level 2 adaptive cache) in ZFS:
[http://fxr.watson.org/fxr/source/cddl/contrib/opensolaris/ut...](http://fxr.watson.org/fxr/source/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c#L4245)
. Most of the ZFS codebase is extremely well documented due to its comments,
and it helps a lot having the documentation _in the code_, to provide context
(something that commit logs, by default, don't).

~~~
davidgerard
The hard part IME is keeping your audience in mind.

Your future self? Your coworker who might be hired next month? Your past self
who would have wished to read said comment?

Who is the audience for the comment?

------
gknoy
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.

~~~
gknoy

      Is this a matter of insufficient knowledge of my IDE?
    

After writing this, I realized that it a symptom of exactly that: I'm not
fully savvy with PyCharm/IntelliJ, my Python IDE of choice. I'll run down the
ways I know of to access blame history (in case others don't know it exists).

1) Git -> Annotate: This is almost exactly what I wanted!

This gives a `git blame`-like column next to each code line, and clicking an
item will show a summary of the commit, including both the commit message and
fields modified. I can right click one of the annotations and select "show
diff", and can even see the modifications to any of the files in the commit if
necessary.

Unfortunately, this depends on a commit message explaining why that change
mattered, AND on it being the most recent changes to those lines. Moreover, it
doesn't show the full history of changes for those lines, such as if Alice
added the real change, and Bob just did some whitespace changes.

I can find such information (with effort) using `gitk` or other git log
browsing tools. If the most recent change (shown with git-blame) doesn't have
what I need, I basically need to look at all of the changes for this file,
going back from that commit, and see if I can find the one with a message that
explains why ${hackity-hack} is necessary.

2) Right click a file tab or in the editor, and select Git -> Show History

This is a faster way of browsing the commit messages, but doesn't give enough
information for me to tell whether that change was the one that affected the
lines I care about. If it showed the diff of what the commit changed in my
file, it would be more informative. (This is what gitk shows, for example.)

As far as I can tell, neither of these options is as comprehensively
informative as using `gitk myfile` and backtracking from the commit that `git-
blame` (or PyCharm's Git->Annotate feature) lists as the most recent change to
those lines.

If comments don't describe what Is Happening or What Should Happen, they
should be changed. This is much harder than maintaining unit test coverage,
but I think the information is still very valuable to have in code comments
rather than solely in commit messages. (It is certainly valuable to have them
in commit messages as well, of course, as that documents how you felt at the
time.)

------
joseraul
> 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.

------
RyanMcGreal
With respect to TODOs and FIXMEs in the comments, I use them to mark places I
know I will need to go back. Before calling a piece of code done, I grep for
TODO and FIXME to make sure I haven't left anything unfinished.

~~~
gknoy
I agree. I use TODO indicators as a code smell for things I forgot to add
before making my pull requests, but I also see them as valuable reminders for
future needs.

Sometimes there's a quick-but-less-generalized way of solving something, and
you know there's a more elegant way of doing it, but for now you need to get
something working/fixed and don't want to make sweeping changes.

Your immediate change might be to add a way for users to see that X is
finished. A useful TODO message would be something which explains what you're
doing

    
    
      # Tell users when their Solve-My-Foo is done
      # (Currently done in an ad-hoc way)
      # TODO: Add a notification infrastructure so that 
      #    arbitrary components of the apps can present
      #    notifications of Important Things
      #    e.g.:
      #    - celery job is done
      #    - a thing you've subscribed to has changed
      #    - a report/download/etc is ready to be retrieved
    

Building such a system might be overkill right now, if this part of your
system is the only one that uses it, but a TODO message helps me see (the next
time I make changes to it, or try to extend it, or use it as a template for
reimplementation elsewhere) that there's more than one place we need this
feature, and so it might make sense to plan/do those more comprehensive
changes.

------
probablyfiction
It would have been helpful if the author explained how this has benefited him
in concrete ways, rather than asserting that his way is best and leaving it at
that.

~~~
lordbusiness
Indeed. "I hate reading articles that make abstract arguments, so enough
bloviating" Author then proceeded to do exactly that.

The single item I agreed with was the blocked out old code that will never be
used again - source control handles that.

Otherwise, what a load of nonsense.

------
rubyn00bie
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?

------
fredophile
I really dislike his notion of using the commit logs as the correct place to
explain your code. First, it means that any time I need to find out why
something is being done in code I need to leave the code and switch from my
IDE to my source control. Second, most tools like blame will only show the
most recent change. Minor fixes and changes happen all the time. What are the
odds that the explanation I want will come up instead of something like "Fixed
a typo". Third, what about code from other branches and external code drops?I
may be using code from somewhere else and not have access to the full history
of changes, just stable versions. Fourth, what happens when you change source
control? If all of the comments are in the Git commit logs that'll be a big
problem if the company switches to Perforce.

Instead of just making meaningful commit logs part of the code review process
why not make "reading the comments" part of the code review process?

------
snake_plissken
As someone who is currently debugging a codebase that was written 5+ years ago
by persons long gone, please, comment wherever you think it would be
appropriate, and then some more. There are only a few things more hellish than
being able to read the code and know what it does, while not being able to
understand why something was coded a certain way.

------
wehadfun
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

~~~
Nerdfest
TODOs are presented in task lists, etc, by many IDEs. For me, they're included
in my workflow, not forgotten.

------
DanielBMarkham
Thanks Andrew.

I think you missed a key point, or maybe I overlooked it while scanning.

Because modern programming languages allow very long variable and method
names, you should be able to construct a "grammar" in your code. That is, the
code itself should be able to be read -- and when you read it, it tells you
what it does.

So "if CustomerHasALargeAccount then WriteThemASalesLetter()" not only is a
piece of code, it's also directly explaining what's going on. As long as you
make sure your names are long and explicit enough, your comments and code
become one. (Except for some oddball cases)

I like comments while I'm constructing my thoughts. Once I've coded it, I
delete the comments. I can also see using comments as sort of a mnemonic for
future work.

A lot of good points here. Keep up the blogging!

------
ggchappell
Nice article, but one passage gave me the willies:

> If you’re writing a public API for a library that’s getting exported to the
> world, or to a bunch of developers at your company, it may be easiest to
> maintain the documentation for that API in code comments.

An important idea that I always try to keep in mind when coding is that the
_me_ of a few months from now might as well be a different person. All those
things I understand so well today, will be no more obvious to me next year
than they will for some other person.

So I think that if I follow a methodology that makes a distinction between
code for me and code for others, then I'm Doing It Wrong.

------
collyw
I can't see why he thinks his second example is any more readable than his
first. The commented version explains what will happen. The second version
requires at least some knowledge of the context.

There are plenty of times when I have been given bits of other peoples code
and told to make them work. All I have is an error message. Usually very
little context about what the program is doing or why. In these cases,
comments make a world of difference.

Even in my own code, looking back on it after 6 months. Sure, I can read the
code and understand it, but it will take me 5 times longer than if there is a
helpful comment near each step.

------
chillingeffect
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.

