Code Tells You How, Revision History Tells You Why

I had a little rant on commenting style and it seems that Jeff Atwood has beat me to it. Well, here's my rant anyway.

There are three different components I care about when reading code: "The How", "The Why", and "The What".

"The How" is the code. The very essence of code is to tell the computer how to do something. It's not 'what' to do, because that is open to interpretation, you might be trying to do something (what) and telling the computer to do something else (how). I don't need comments for the how, I can read the code. An example of a bad "how" comment is:

/* assign 0 to all flags */
for (i = 0; i < length; i++) things[i]->flag = 0;

I can see you're setting all flags to zero. Thanks for the non-information.

"The What". This is the interpretation of "The How". Basically, it's what that code is trying to accomplish. For instance:

/* clear all flags */
for (i = 0; i < length; i++) things[i]->flags = 0;

At least now I know what zero means. But it still doesn't tell me a lot. As a matter of fact, it's rare when I need to know "The What". If I need comments to know "The What", the code is probably poorly written. I should be able to infer "The What" from "The How".

"The Why". This is what I really care about. Why is this piece of code like this? Why not some other way? What was in the programmer's mind when he wrote that piece of code? What alternatives were considered and discarded? Why were they discarded?

/* We're on a deadline and I don't understand why the flags
 * array is getting corrupted. Clearing all flags seems to
 * fix it, but there is a deeper issue here that needs
 * investigating
 */
for (i = 0; i < length; i++) things[i]->flags = 0;

Ah, much better now. I've gained some insight into the programmer's mind, so later when I'm working on the code I can make sense of this line. Basically, I can answer the question: "Why was this added?"

But wait, there are problems with this approach.

First of all, you're using up 5 lines of comments for 1 line of code. That's verbose, but not the worst of the problems.

What's worse is that if I'm just skimming the code because I'm trying to solve some other problem for which I don't really care about this particular line, you're going to make me stumble. I'm going to read that comment, remember that I once knew of a way in which the flags could get corrupted and spend the next 1/2 day chasing an issue that has a workaround. Your nice comments are on my face when I don't need them.

Even worse, this style also leads to comments needing maintenance. The code will change, and the comment won't be updated with the change. You'll get

/* We're on a deadline and I don't understand why the flags
 * array is getting corrupted. Clearing all flags seems to
 * fix it, but there is a deeper issue here that needs
 * investigating
 */
for (i = 0; i < length; i++) {
 if (things[i]->flags & FLAG_INIT) things[i]->flags = 0;
}

Whoa there buddy! You're saying that flags were getting corrupted and now you're using them? How can you trust them? Either the comment is wrong or the code is wrong.

What probably happened was that some programmer didn't read the comment (or read it but forgot to change it) and found a convenient place for putting his change. Now understanding this piece of code has been made harder than it should be.

And all of this is just looking at code statically. Code is organic, it grows, it changes, it evolves.

There aren't a lot of comments in the BitKeeper source base because we rely heavily on the Source Control system to answer the "Why" questions. So you'll see the above code as:

for (i = 0; i < length; i++) things[i]->flags = 0;

No comments whatsoever. However, if you need to understand what was going on, you can get the annotated version:

ob 1.123.21: for (i = 0; i < length; i++) things[i]->flags = 0;

and see the comments for the ChangeSet 1.123.21:

ChangeSet@1.123.21, 2006-07-21 10:45:34-07:00, ob@bitmover.com +1 -0
  Fix bug 2004-10-21: app crashes when restarting
src/main.c@1.23, 2006-06-29 19:40:46-07:00, ob@bitmover.com +1 -0
  We're on a deadline and I don't understand why the flags
  array is getting corrupted. Clearing all flags seems to
  fix it, but there is a deeper issue here that needs
  investigating

Now the information is there, but it's not in my face unless I need it.

Note that when looking at code this way, the question "What changed?" is trivially answered by the diffs and therefore doesn't need to be answered by the comments. E.g. the following are really bad comments:

src/main.c@1.23, 2006-06-29 19:40:46-07:00, ob@bitmover.com +1 -0
  Add code to clear the flags

Well, duh! I can see that from the diffs!

@@ -274,6 +278,7 @@
  things = getThings(x, y, z);
  length = getLenght(things);
+ for (i = 0; i < length; i++) things->flags = 0;
  doStuff(things);
}

I can see that you added code to clear the flags, why did you do it? That's the interesting bit.

Technorati Tags: BitKeeper, Programming

Spotlight (part deux)

After reading some comments about how Spotlight just works for other people, and talking with some friends that have Spotlight turned on, I started doubting that it was so bad. It turns out that when I got the MacBook Pro, I transfered about 40 GB of stuff from my old laptop, and apparently it also transfered a corrupt Spotlight Index.

So after running:

# mdutil -E /

and turning Spotlight back on, I'm no longer seeing the performance problems I had. All thanks to the comments. And I didn't think anybody read this blog :)

Spotlight must die (or get its act straight)

I just got a new MacBook Pro, and had it for about a week. What I discovered was that the machine wasn't as snappy as I thought it would be. It's a CORE DUO for Christ's sake, it has not one, but TWO cores, it has 2 GB of freaking RAM! It should be the fastest box around! But no, it's slow and every once in a while it just locks up for about 5 seconds.

After some fun with fs_usage trying to figure out what was going on, I found the culprit: Spotlight. See, spotlight interacts VERY VERY badly with the file system cache, and since Mac OS X has a unified cache, with the VM cache. This means that while the OS is trying really hard to keep your working set in memory, spotlight (a.k.a. mdimport) is indexing stuff and therefore causing unnecessary flushes of the cache.

The solution? Turn it off. Just run 'mdutil -i off /' and edit /etc/hostconfig to set "SPOTLIGHT=-NO-" and be done with it. Or if you don't want to even type that, get Spotless and click away. Go ahead, I'll wait.

The problems? Mail no longer searches, and no easy and quick way to launch applications. I installed QuickSilver for the application launching part, and will figure out if I can find an alternative for searching mail. But it's worth it.

My machine is SUPER FAST now, and the disk is quiet all the time. Processors are mostly idle if I'm not doing anything, ah the sound of a high performance machine...

How badly does Google Desktop Search hinder performance under Windows?

Technorati Tags: macos x