What is Well-Commented Code?

WannaBeGeekGirl queries: "What exactly is well-commented code anyway? Can anyone suggest resources with insight into writing better comments and making code more readable? After about six years in the software development industry I've seen my share of other people's code. I seem to spend a lot of time wishing the code had better (sometimes _any_) comments. The comments can be frustrating to me for different reasons: too vague, too specific, incoherent, pointing out the obvious while leaving the non-obvious to my imagination, or just plain incorrect. Poorly or mysteriously named variables and methods can be just as confusing. In a perfect world everyone would follow some sort of coding standards, and hopefully those standards would enforce useful comments. Until then, any suggestions for what you, as a programmer, consider to be good/useful/practical comments? Any suggestions for what to avoid? Also, I usually work with C++ so any resources/comments specific to that language would be too."

Comments

It's been a long time but..

[NewtonsLaw]

It's been quite a while since I wrote any significant amount of code but after spending far too many years cutting code too early in the development process I eventually woke up to the fact that coding is the *last* thing you do (apart from testing and debugging that is).

First-up you need a good spec -- and the spec should include the user-interface details to the extent that you could actually write the user-manual from that spec.

Indeed -- if you can't write the user-manual from the spec then the spec is incomplete.

From the spec the programmer should develop the structure of the code in another document.

That structure document is repeatedly refined in a top-down process until you (eventually) reach a point where you're actually cutting code.

I was always surprised just how much easier it was when the code was written as the lowest level of the structure documentation.

Not only could you comment out the program structure document so that the compiler would ignore it -- but you ended up with absolutely accurate and comprehensive documentation built into that source.

Project managers love this technique (and when I was in a project management role I demanded it of my team) -- it ensures that technical and end-user documentation are no longer the bits that get left until last and thus are either very shoddily thrown together or, if the project goes really over-budget, not produced at all.

Of course, as we all know, there's a huge amount of temptation to just leap into coding at the earliest possible stage and leave the documentation until later -- because some stupid managers use number of code-lines completed as a metric of project performance -- duh!

If you're smart and use good tools you can selectively collapse and expand the in-source documentation so that when you're trying to get familiar with a module that someone else has written, you can descend down the structure tree one level at a time without the meaning being diluted by stuff that is at a lower level.

Unlike the days of interpreted BASIC, there's very little overhead involved in integrating documentation and code these days -- so there's no excuse not to do it.

If required, the documentation can be automatically extracted from the source -- but by keeping the master copy in the code it becomes easier to ensure synchronization as changes and updates are made during the lifecycle of the project.

Document the function's contract

[IvyMike]

Take a look at this function, and tell me if there's a bug:

void foo(void) {
int* x = 0;
int y = *x;
}

Easy, the bug's the SEGV, right? Take a look at the same function, this time with comments:

// Function: cause_segv
// Description: Causes a SEGV for testing purposes
void cause_segv(void) {
int* x = 0;
int y = *x;
}

The point? A bug is unwanted behaviorm, but that only makes sense if you've defined what the correct behavior is. My example is trivial, but often this is a real concern. Function "bar(int,int)" returns null whenever one of the arguments is negative--is that a bug or a feature? Your function has a goal in life, a contractual obligation to do something; make sure it's clear what that something is.

Note that if you choose good function and good variable names, a simple one or two line comment at the beginning is usually sufficient to document whe function's intended behavior.

I also find that an "assert()" or two on the arguments at the top of the function makes it clear what values the function accepts, and which one the function doesn't handle. It's an easy way to document the contractual obligations of the function.

Stuff not to put in comments is stuff that's easily devised from the code. Check this out:

// Function: square
// Inputs: int x
// Outputs: int
// Used by: pythagorean(int,int)
// Description: returns x squared
int square(int x) { return x*x; }

Did the "Inputs" or "Outputs" add any value? That information appears again, two lines below in the function definition, and it's guaranteed to be correct there (unlike the comment which will be out-of-date and wrong when we change "square" to work on longs). The "Used by" might have added some value, if it was correct, but as it turns out it's out of date, and 15 other functions now use "square". Any information better derived looking at the code should be left off. Any information which can be better found using "grep" or "find in files" should be left off. Any information that will probably be out of date at some point should be left off. Heck, in this situation even the description is probably extra verbiage, since it doesn't really help anyone. (I'd probably put it in out of habit anyway, though...so sue me:)

Re:Variable Names

[Grab]

Sometimes, from other ppl. If I see it, it goes right back in review, and I won't pass the review until the fuckwit responsible has removed them. If you're writing code for yourself, then fine, please yourself. If you're writing code that anyone else will see, *especially* the customer, then hell no.

Thing is, there's two essential things that a reviewer/maintainer has to understand about a program: what it does; and why it does it. It should be possible to work out the first one of these just from the code, so long as the variables and functions are named sensibly. The second can be worked out from code with some effort, or the coder can add comments to explain why they're doing things that way and make it easier for maintainers.

But if someone has deliberately given all the variables names which don't reflect what they do, then it's utterly impossible to work out what the code is doing, and it's therefore also impossible to work out why it's doing it. So the code is unmaintainable - it isn't possible for anyone else to pick it up and work out what it does, except with massive work. If in 6 months time your company says "oh, we've got this code we can use with slight modifications, let's quote 1 month to do this contract" and then they find out you've made the code utterly obscure, then they'll crash and burn. And if that happens, the company *will* fire (or at least formally discipline) the person who wrote the original code, bcos they've been grossly negligence in doing their job. And you can kiss goodbye to any reference from them, so you'll be SOL in finding your next job.

Grab.