The worth of comments

[Richard Parker]

My experience is that comments in Python are of relatively low
usefulness. (For avoidance of doubt: not *zero* usefulness, merely low.)
I can name variables, functions and classes with sensible, self-
documenting names. Why write:

x = get_results(arg) # x is a list of 1 or more results
[... much later]
for y in x:
# process each result in turn
do_something_with(y)

(It occurred to me that I should use a specific subject for this discussion.)

I'm less inclined to use comments on each line, or selected lines, but rather use block comments instead. They require more thought and time to write; however, the intended functionality of the code that follows can be describedin full.

 

[Roy Smith]

My experience is that comments in Python are of relatively low
usefulness. (For avoidance of doubt: not *zero* usefulness, merely low.)
I can name variables, functions and classes with sensible, self-
documenting names. Why write:

x = get_results(arg) # x is a list of 1 or more results
[... much later]
for y in x:
# process each result in turn
do_something_with(y)

(It occurred to me that I should use a specific subject for this discussion.)

I'm less inclined to use comments on each line, or selected lines, but rather
use block comments instead. They require more thought and time to write;
however, the intended functionality of the code that follows can be described
in full.

Over the years, my use of comments has evolved. I was taught, "You
should comment your code". Eventually, I came to realize that the real
mandate is, "You should make it easy to understand your code". Comments
are just one possible tool to help achieve that goal.

Some things that help code be understandable are:

* Using good data structures

* Using good algorithms

* Breaking the code up into manageable pieces (i.e. functions, classes,
methods), each of which encapsulates a single concept

* Using descriptive names for variables (and functions, classes,
methods, etc)

All of those fall under the "self-documenting code" umbrella. But,
while those things help, I find it's almost always a good idea to
document interfaces, i.e. functions. What the arguments are (not just
their types, but valid value ranges, and what they mean). What the
function returns. What error conditions are possible. And, in general,
what the dang thing does. In other words, enough information to use
(and test) the function to its fullest extent without ever having to
look at the source code. This stuff tends to go in a big block comment
at the beginning of the function.

Now, what makes Python interesting in this regard is that the big block
comment becomes a doc string. You write exactly the same stuff, except
now things like help() can get at it, and things like doctest can do
even more interesting things with it (I don't actually use doctest, but
I do appreciate its coolness).

Comments scattered throughout the code tend to be warnings about tricky
stuff, references to classic algorithms, references to ticket tracking
systems, and the like. Sometimes it's an explanation of what the next
bunch of lines of code are doing, although if you've got a lot of those,
that's a hint that maybe you need to be refactoring instead. Sometimes
I'll drop in suggestions to future maintainers like, "consider
refactoring with with perform_frobnitz_action()", or even, "Don't change
this without first talking to Fred!"

 

[Chris Angelico]

 Sometimes
I'll drop in suggestions to future maintainers like, "consider
refactoring with with perform_frobnitz_action()"

Usually, I address future-me with comments like that (on the
assumption that there's nobody in the world sadistic enough to want to
maintain my code). But I never name future-me, the comments will be
addressed to "the subsequent maintainer" or somesuch. I do assume,
though, that future-me has forgotten everything about the code, and
since past-me left some comments that current-me has read, I think the
assumption is fairly accurate. (Did I *really* write that code? It has
my name on it.....)

Chris Angelico

 

[Roy Smith]

(Did I *really* write that code? It has my name on it.....)

Most version control systems have an annotate command which lets you see
who wrote a given line of code. Some of them are even honest enough to
call the command "blame" instead of "annotate"

And, yes, it's always a rude shock when I stare at some hunk of code,
mutter, "who wrote this crap!?" and fire up annotate/blame only to
discover my name on it.

 

[D'Arcy J.M. Cain]

Ruby has a lot of followers, and I am trying to get excited about it, but
it has succumbed to the same special-characters-as-syntax disease that
killed Perl. Much Ruby code is just unreadable.

What? The recent Perl flame war wasn't enough entertainment for
you? ;-)

 

[Grant Edwards]

I've seen plenty of comments who's usefulness was not zero. It was
less than zero.

I am largely in agreement with this position (including the ???not *zero*
usefulness??? caveat).


This I disagree with. If a section of code is interesting enough to
deserve an explanation, then it is better to capture it in a
helpfully-named function with its doc string having the explanation.

I consider docstrings to be the same as comments, so there's not
really much disagreement.

 

[Irmen de Jong]

I've seen plenty of comments who's usefulness was not zero. It was
less than zero.

Someone once taught me, "There is one thing worse than having no
comments in the source code: having incorrect (or 'lying') comments in
the code."

Grant, I guess you hint at such comments?

Irmen.

 

[Grant Edwards]

Someone once taught me, "There is one thing worse than having no
comments in the source code: having incorrect (or 'lying') comments
in the code."

Grant, I guess you hint at such comments?

Yes.

When trying to find a bug in code written by sombody else, I often
first go through and delete all of the comments so as not to be
mislead.

The comments reflect what the author _thought_ the code did
_at_some_point_in_the_past_. What matters is what the code actually
does at the present.

 

[Prasad, Ramit]

(Did I *really* write that code? It has my name on it.....)

Damn those ninja programmers who stole your name and coded something!


Ramit



Ramit Prasad | JPMorgan Chase Investment Bank | Currencies Technology
712 Main Street | Houston, TX 77002
work phone: 713 - 216 - 5423



This communication is for informational purposes only. It is not
intended as an offer or solicitation for the purchase or sale of
any financial instrument or as an official confirmation of any
transaction. All market prices, data and other information are not
warranted as to completeness or accuracy and are subject to change
without notice. Any comments or statements made herein do not
necessarily reflect those of JPMorgan Chase & Co., its subsidiaries
and affiliates.

This transmission may contain information that is privileged,
confidential, legally privileged, and/or exempt from disclosure
under applicable law. If you are not the intended recipient, you
are hereby notified that any disclosure, copying, distribution, or
use of the information contained herein (including any reliance
thereon) is STRICTLY PROHIBITED. Although this transmission and any
attachments are believed to be free of any virus or other defect
that might affect any computer system into which it is received and
opened, it is the responsibility of the recipient to ensure that it
is virus free and no responsibility isaccepted by JPMorgan Chase &
Co., its subsidiaries and affiliates, as applicable, for any loss
or damage arising in any way from its use. If you received this
transmission in error, please immediately contact the sender and
destroy the material in its entirety, whether in electronic or hard
copy format. Thank you.

Please refer to http://www.jpmorgan.com/pages/disclosures for
disclosures relating to European legal entities.

 

[Irmen de Jong]

Yes.

When trying to find a bug in code written by sombody else, I often
first go through and delete all of the comments so as not to be
mislead.

The comments reflect what the author _thought_ the code did
_at_some_point_in_the_past_. What matters is what the code actually
does at the present.

I'm going to share this thread, and the funny slideshow about Uncomment your code, with
my team at work
We're not a Python shop so I'm probably the only one reading this, but as usual there is
a lot of wisdom going on in this newgroup that is not only applicable to Python.

Irmen

 

[python]

Irmen,

I'm going to share this thread, and the funny slideshow about Uncomment your code, with my team at work We're not a Python shop so I'm probably the only one reading this

Same here!

but as usual there is a lot of wisdom going on in this newgroup that is not only applicable to Python.

+1 QOTW

Malcolm

 

[Roy Smith]

When trying to find a bug in code written by sombody else, I often
first go through and delete all of the comments so as not to be
mislead.

I've heard people say that before. While I get the concept, I don't
like doing things that way myself.

_at_some_point_in_the_past_. What matters is what the code actually
does at the present.

I don't look at it that way. Most of the comments I write are to
document interfaces, i.e. what the code is supposed to do. That's the
contract between you and the code. If you call me with arguments that
meet these conditions, this is what I promise I'll return to you (and
what side effects I'll perform).

One reasonable definition of a bug is something the code actually does
which differs from the documented interface. Unless you know what the
code is supposed to do, how can you possibly look at it and say whether
it has a bug or not? For example, take this function:

def foo():
l = [1, 2, 3]
return l[3]

Is this code correct? I'll bet most people would look at this and say,
"I'm not sure what he had in mind, but whatever it was, this has to be a
bug because he's indexing past the end of the list". Well, what if I
showed you the interface contract:

def foo():
"Raise IndexError. This is useful as a testing fixture."
l = [1, 2, 3]
return l[3]

Now it's obvious that the function does exactly what it's supposed to do
(even if it's not the best way to do it).

 

[Chris Angelico]

def foo():
  "Raise IndexError.  This is useful as a testing fixture."
  l = [1, 2, 3]
  return l[3]

A quite useful thing, on occasion. I have a couple of variants of
this, actually. In one of my C++ programs:

extern char *death1; extern int death2; //Globals for killing things with

// further down, inside a function:
case "death1": *death1=42; break; //Die by dereferencing NULL
case "death2": return 42/death2; //Die by dividing by zero

They were designed to verify the parent-process code that was meant to
catch process termination and identify the cause, so I wanted two
quite different ways of blowing up the program. (The variables were
extern and defined in another file to ensure that the compiler
couldn't outsmart me with a compilation error.)

In the Python code, that would be unnecessary with the *list* type,
but it might be of value with your own class (eg subclass of list).
Although, I'd put that sort of thing into a dedicated unit testing
section, where everyone _knows_ that you're trying to break stuff.

Chris Angelico

 

[Irmen de Jong]

I've heard people say that before. While I get the concept, I don't
like doing things that way myself.


I don't look at it that way. Most of the comments I write are to
document interfaces, i.e. what the code is supposed to do. That's the
contract between you and the code. If you call me with arguments that
meet these conditions, this is what I promise I'll return to you (and
what side effects I'll perform).

I don't see how that is opposed to what Grant was saying. It's that these 'contracts'
tend to change and that people forget or are too lazy to update the comments to reflect
those changes.

The comments you are writing, saying what the code is supposed to do, are only saying
what the code is supposed to do at the moment in time that you were writing the comment
and/or the code, are they not?

One reasonable definition of a bug is something the code actually does
which differs from the documented interface. Unless you know what the
code is supposed to do, how can you possibly look at it and say whether
it has a bug or not? For example, take this function:

def foo():
l = [1, 2, 3]
return l[3]

Is this code correct?

Unit tests should tell you.

I'll bet most people would look at this and say,
"I'm not sure what he had in mind, but whatever it was, this has to be a
bug because he's indexing past the end of the list".

I do agree that reading just that piece of code without other information, makes me
think that it is fishy. But:

Well, what if I
showed you the interface contract:

def foo():
"Raise IndexError. This is useful as a testing fixture."
l = [1, 2, 3]
return l[3]

Now it's obvious that the function does exactly what it's supposed to do
(even if it's not the best way to do it).

A month later the requirement changes: it should raise a ZeroDevisionError instead.
Someone modifies the code but leaves the comment (for whatever reason).

def foo():
"Raise IndexError. This is useful as a testing fixture."
return 1//0

Unless there is a way to force people to update the code comment as well (which I'm not
aware of.. Doctests? dunno...), you now have a comment that lies about the the intended
working. In my opinion that is worse than what we had before (the function without
comment).


That being said, I do write code comments myself. Some of them are like notes, directed
to future-me or other future readers (remember that we do this because blabla, don't
change this to such-and-such because it will probably break xyz) and some of them are
stating the contract of the code (like you wrote above, about documenting interfaces).
I always try to avoid writing comments that duplicate the working of the code itself in
pseudo code or text phrases. But humans make mistakes and forget things so after enough
modifications my code probably contains lies as well...


Irmen de Jong

 

[Gregory Ewing]

I don't see how that is opposed to what Grant was saying. It's that these 'contracts'
tend to change and that people forget or are too lazy to update the comments to reflect
those changes.

However, I can't see that deleting the comment documenting the
contract can be the right response to the situation.

If the contract comment doesn't match what code does, then
there are two possibilities -- the comment is wrong, or the
code is wrong. The appropriate response is to find out which
one is wrong and fix it.

If you simply delete the comment, then you're left with no
redundancy to catch the fact that something is wrong.

 

[Irmen de Jong]

However, I can't see that deleting the comment documenting the
contract can be the right response to the situation.

If the contract comment doesn't match what code does, then
there are two possibilities -- the comment is wrong, or the
code is wrong. The appropriate response is to find out which
one is wrong and fix it.

Fair enough.

Certainly I won't be deleting every source code comment encountered from now on, but I
do think we should keep in mind the risks already mentioned. In some situations I can
very well imagine it is best to simply delete any comments and go with just the code.

If you simply delete the comment, then you're left with no
redundancy to catch the fact that something is wrong.

You are right, if you don't have a Unit test for it.
Then again, faulty unit tests are a problem in their own right...

Irmen de Jong

 

[Gregory Ewing]

You omit the common third possibility: *both* the comment and the code
are wrong.

In that case, the correct response is to fix both of them.

 

[Grant Edwards]

In that case, the correct response is to fix both of them.

Only as a last resort. IMO, the best option is to fix the code so it's
purpose and operation is obvious from the code, and then delete the
comment.

 

[Roy Smith]

Only as a last resort. IMO, the best option is to fix the code so it's
purpose and operation is obvious from the code, and then delete the
comment.

This is a plausible(*) strategy for internal use software where all
users have easy access to all code which depends on yours and are free
to change interfaces willy-nilly. That's not always the case. Even on
open-source projects, having stand-alone documentation is critical for
usability, and certainly having stable interfaces is critical if you
expect people to adopt your system and build on it.

(*)And, even in the case where it's internal code and everybody on the
project has full and unfettered access to all the source, documenting
interfaces adds to usability. I've seen plenty of code which looks like
this (pseudo-code):

Class1::f1() {
return Class2::f2();
}

Class2::f2() {
return Class3::f3();
}

Class3::f3() {
return Class4::f4();
}

If you're trying to figure out what type of object f1() returns, you've
got to chase down a long string of, "Well, f1 returns whatever f2
returns, and f2 returns whatever f3 returns, and f3 returns whatever f4
returns, and...." Each step in that process might involve figuring out
just where the heck the code for ClassN is. And Cthulhu help you if
some step along the way involves an indirectly referenced class or
function so you can't even grep the source tree for the name you're
looking for.

 

[Alister Ware]

However, I can't see that deleting the comment documenting the contract
can be the right response to the situation.

If the contract comment doesn't match what code does, then there are two
possibilities -- the comment is wrong, or the code is wrong. The
appropriate response is to find out which one is wrong and fix it.

If you simply delete the comment, then you're left with no redundancy to
catch the fact that something is wrong.

"if the comments & code disagree then both are probably wrong"