?

Log in

current entries friends' entries archives about me Previous Previous Next Next
Comments - cellophane
the story of an invisible girl
renniekins
renniekins
Comments
I was talking to the professor of my C++ class awhile back, and he was looking at my code.

"I'm actually pleasantly surprised by your code," he told me. "It's very good, very readable. The only feedback I have, and take this how you want... you don't use many comments. Actually, any comments."

I grinned. I was wondering when this would come up. That's right baby, that's the Agile Way!

I didn't answer that way of course; I was more polite. "Well, that's a kind of philosophy where I work you see. The idea is that comments will get stale. When code is changed, the comments may or may not change along with it. Instead the code itself should be clean and readable, thus creating its own documentation. You refactor it; you use very informative method names and variable names to make it understandable."

He wasn't entirely convinced. "Well your code is very readable. It's just there aren't many comments. Take that how you will."

I thanked him -- but I wasn't entirely convinced either.

The next assignment I turned in was a bridge-dealing exercise that I worked on during the drive back from Mackinac. I generously gave him a few comments. Since (for the sake of keeping my deliverables clean) each exercise is in its own single source file, I provided a comment on the top of each class definition for visual separation.

My pride and joy though, is the single informative comment I included in the code. It can be seen below.


/* MAIN */

int main() {
Deck deck;
deck.display("The clean deck: \n");

for (int i=0; i<7; i++) {
   // shuffling a deck past 7 times is shown to be statistically insignificant
   deck.shuffle();
}
deck.display("\nThe shuffled deck: \n");

deck.dealCardsTo("Rennie", "Sandra", "Rob", "Lisa");
deck.display("\nThe dealt deck: \n");

return 0;
}

Tags:

read 20 comments | talk to me!
Comments
pstscrpt From: pstscrpt Date: October 12th, 2007 07:58 pm (UTC) (Link)
Around here, the names seem to be every bit as likely to get stale as the comments.
greyyguy From: greyyguy Date: October 12th, 2007 08:11 pm (UTC) (Link)
Agile encourages not using comments? I didn't know that.
renniekins From: renniekins Date: October 22nd, 2007 01:05 am (UTC) (Link)
Well, more specifically: agile encourages refactoring and tweaking code, and using informative method and variable names, so that it is so clean and informative that comments are not necessary.
(no subject) - davehogg - Expand
From: tlatoani Date: October 12th, 2007 08:32 pm (UTC) (Link)
I love that!
jer_ From: jer_ Date: October 12th, 2007 08:44 pm (UTC) (Link)
I am a moderate commenter. I don't comment MORE because of exactly that reason, I don't change comments often when I change code.

What that means, though, is when I comment, it's typically because I screwed something up or I made such a confusing clusterfuck of something that nobody will untangle it later.

I tend to be more proud of those moments than I should be. :)
eviljohn From: eviljohn Date: October 12th, 2007 08:55 pm (UTC) (Link)
You extreme girl you.

I'm familiar with both arguments, it's interesting how times change though. Back in the early C days most folks got criticized for not using enough comments, or *gasp* non at all. These days it's the goal.

Personally, I don't care either way, as long as someone new can sit down read the conde and quickly figure whatit's doing. I spent far too much time in my career reading bad code, *with and without* comments.
kyril From: kyril Date: October 13th, 2007 05:05 pm (UTC) (Link)
I think it's because people are more comfortable with/used to/willing to type/able to autocomplete longer identifiers. Code wasn't nearly as self-documenting then.

I usually only comment something when I feel it's non-obvious or counter-intuitive, or when I'm documenting a change/fix or need to refer to a requirement number.

When you change the functionality, of course, it's essential to fix the self-documentation as needed. And when reviewing code changes, be sure that new code doesn't separate comments from the code they apply to. That's probably the easiest way for comment rot to start.
specialagentm From: specialagentm Date: October 13th, 2007 07:40 pm (UTC) (Link)
I agree. Knowing where to put the comments is as important as the commenting itself. If the method is at all "tricky", comment the method right at the top. If there's a strange chunk of code somewhere in a method, or some dependency on the state of something else in the code tree, comment right there where the code linkage happens.

Hell, comment the whole damn class if you need to. I've had some very good code constructs that worked but had a definite contract that needed to be followed to use them, one that might not be intuitive to a future coder on the team -- if I didn't comment that, it could have been too easy for a new team member (even one who was very smart) to inadvertently break the code or for them to just say "oh hell, I'll just rewrite this, since I don't understand it". To me, that breaks all bounds of sanity and leads to wasted effort. If you want to rewrite my code fine -- but at least understand what it's doing first and make an informed decision rather than a knee-jerk reaction.
thelifeofbrian From: thelifeofbrian Date: October 13th, 2007 02:35 am (UTC) (Link)
That was quite readable. I always thought my code was really readable, too. Then, someone else would take a look at it while I was away, and when I got back they'd ask me what the <explicative deleted> my code was supposed to do. (Regular expressions are great for provoking that reaction).

So, I switched coding languages for tools, to something more readable and easier to understand by my colleagues. Now, when they look at my code, when I get back, they ask me what the <explicative deleted> my code was supposed to do.

Good think I switched coding languages, huh?
cannibal From: cannibal Date: October 13th, 2007 04:30 pm (UTC) (Link)
Yeah, regex can be incredibly terse, and what's the point of writing ten lines of comments for one line of code? It'd be like comments in APL, your whole program might be 4 lines long, if you can't understand the way APL works you might as well just give up.

Indirect variables are pretty good, too.
kyril From: kyril Date: October 13th, 2007 05:08 pm (UTC) (Link)
s/APL/Perl/g and it's almost as bad. TMTOWTDI leads to every piece of a program doing the same it in a different way in very short order. Even when it's just me writing it.
cannibal From: cannibal Date: October 13th, 2007 04:54 pm (UTC) (Link)
My favorite comment of all time was "here a miracle occurs" in the text editor for FMEAplus... I hated it, the users kept adding more features on it, and by version 2.0 the damned thing worked, but between right justify, tabs, indents, and bullets, it wasn't pretty. I left the comment to warn future generations not to touch it... and got one amused visit from one of the programmers on my team.

Fortunately, for version 3.0 we started a new source tree using the XVT dual UNIX/Windows framework, and I dictated that we would use the XVT editor from the library without modifications. My new team was in Germany... so half of the comments and variable names were in German!
specialagentm From: specialagentm Date: October 13th, 2007 07:35 pm (UTC) (Link)
I totally agree with your professor. Stale comments are bad comments. If you can't update the comments when you update the code, you're doing something wrong.

Comments are there because they are... comments. They don't impact the code performance, so they can be as long as you want, explaining why you did some strange combination of code operations that are efficient, but hard to read.

Wouldn't you want a comment for something like this?

MapFunctor alertIdChecker = new MapFunctor() {
  public AIAlert accept(AIAlert alert)
  {
    if (alert.getAlertId() < 500)
    {
      return alert;
    }

    return null;
  }
}
String alertIDs = MSSCollectionsUtil.join(MSSCollectionsUtil.map(alertIdChecker, (MSSCollectionsUtil.collect(alertList, "alertId"), ","));



Like:

// Use the collect method to strip the alert ids from our alert list,
// joining them with a comma in-between each. Reject alert ids
// above 500 using a MapFunctor, because those are just for
// testing purposes. Once testing
// is over, remove that operation and this comment

Nope, give me comments any day. Bad comments might suck, but so does bad code -- why throw the baby out with the bathwater? I think this is where agile crosses over into cowboy programming and actually does a very poor job of supporting new people who come onto a team.
renniekins From: renniekins Date: October 21st, 2007 08:35 pm (UTC) (Link)
I really wanted to refactor that code to show you how I'd have written it, but I kept finding myself unable to. Eventually I realized that it's because there are 4 left parentheses and only 3 right ones.

specialagentm From: specialagentm Date: October 21st, 2007 11:43 pm (UTC) (Link)
Refactoring presumes understanding first. How do you refactor code you don't even understand the intent of? What do you do when the code can't speak for itself?

I think there are far too many edge cases out there for commenting to be thrown out like the proverbial baby and bath water. Of course, like anything else, you need to know how to properly use it. Much like "goto", self-modifying code, and the "eval" statement, commenting can either make or break your development.

I just find it silly to eschew a practice that I can prove the utility of with a huge amount of anecdotal evidence. Bad comments can be deleted or modified later -- if good comments aren't captured when the code is being written, the chance to get that key information on intent and decisions might be forever gone.

To keep this on-topic of agile, I find I wrote many more comments when pair-programming then when not. They capture the discussion I'm having with my partner, and we ruthlessly edit the comments as much as we edit the code.
renniekins From: renniekins Date: October 22nd, 2007 12:14 am (UTC) (Link)
In theory, I know its intent because I read your comment, right?

In practice, I avoid refactoring code without solid test coverage, because then even if I don't understand it thoroughly, the tests make sure it doesn't break.

Often refactoring can lead to a stronger understanding of the code, as I reorganize and get a feel for what each bit is doing, when, and why.

If it doesn't come out any clearer, well that's why I have an undo button and a source repository.

And I don't completely eschew comments, only avoid them. I try hard to make my code readable simply as it stands, then only include comments when it is necessary. I'll also frequently start with tons of comments, then gradually make the code do all the things the comments say it will do, removing them only when it's clear.

For example:

int x; // this is the return value

// check for prime numbers
if () {.....many lines of code...}

// now check for square roots
else if () {... more code... }

// if these failed, check invalid data
else if () {...etc, you get the idea...}

might refactor to:

int returnValue;
if (validateData()) {
returnValue = primeNumberOperations();
returnValue += squareRootOperations();
}

Random example of course, but stuff like that. Method calls to replace long chunks of procedural code with comments explaining each chunk's purpose.

But back to your example, does that code really compile with those parentheses? It was weird. I wanted to try to use it for an interesting (for me at least) example, but couldn't balance it.... I didn't actually put it into source code, but still.
specialagentm From: specialagentm Date: October 22nd, 2007 12:28 am (UTC) (Link)
No, it doesn't compile. I was editing down a cut-and-paste example from my code, and somehow mis-deleted a paren.

Anyway, your comment:

"I'll also frequently start with tons of comments, then gradually make the code do all the things the comments say it will do, removing them only when it's clear."

I do that too. In fact, it's pretty much our standard way we do pair-programming on small tasks -- stub everything out, and slowly comments replace code. What comments are left behind are the comments that couldn't be removed, because there was no exact code whose inclusion mirrored what the comment was documenting.
renniekins From: renniekins Date: October 22nd, 2007 12:33 am (UTC) (Link)
Cool. (:
sandygood From: sandygood Date: October 14th, 2007 07:40 pm (UTC) (Link)
Yeah! I love playing cards!
read 20 comments | talk to me!