Monday, March 17, 2008

Do bug references taint code readability?

Sometimes we write code that looks slightly odd or unpleasant, but it works around (or fixes) a bug, a bug which otherwise is perhaps beyond our control (e.g. some interaction issue with a closed source tool). We write the code as best we can to make it self documenting, perhaps adding a short comment to describe why the workaround exists. We write an automated test to protect against regressions. We refer to the bug # in the tests for easier forensics should things go awry. We are fashionable.

Yet we are still worried about someone coming along and shredding our unpleasant fix, forgetting about the regression tests, and reintroducing the bug.

Refer to a bug number in a source code comment?

How fasionable is that? I've googled but couldn't find much on the topic. I've seen it done quite a bit in the Mozilla codebase, and I'm inclined to think it has merit in some cases. I wonder what others think.

Anyway... this blog was inspired by a dojo meeting a few months ago. I let it sit unpublished for a while... until a time when I'm mentally exhausted and just don't care what blatherings I post...

Thanks for reading.

11 comments:

Ryan Zielke said...

Im not a big fan. I think that CMS should help out there if more info is needed.

Simon said...

I don't think it hurts readability, but I don't think there's much value either in putting bug tracker references in the code. In most cases, it should be sufficient to have automated regression tests and a short in-code explanation of why something is being done in a specific way, to warn anyone thinking of rewriting.

david said...

I find that in some cases they're a very compact shorthand to issues that are hard to describe. But certainly they shouldn't be there for every checkin. Checkin logs, however, should have references back to bugs. And bugs ideally should have references back to the checkin that fixed them on closure or verification.

PAStheLoD said...

IMHO, that 6 bytes worth more than a ton of comments.

You can litter your code with long essays why certain parts are the way they are, but adding bug(zilla) references is much cleaner and helpful. (Because searching for a particular checkin sometimes isn't easy.)

Justin said...

I think it depends on the situation.

For normal, mundane bug fixes, mentioning a bug number is usually pointless. No sense in pointing out the obvious.

But if the code change is unusual, mentioning a bug number and brief comment can be a good way to indicate something very non-obvious is happening.

Turned around: code should be mostly empty of comments referring to bug numbers. But when you do see such a comment, it's a sign that there might be something tricky going on, and to tread carefully.

Goat Doctor said...

Yes, absolutely. Referring to bugs in code is a brilliant way for people new to the code to find out about the gotchas -- we do this at my company and it's a lifesaver.

Putting in a paragraph explaining a workaround is good too, but those kinds of explanations tend to get stale as the code changes again, whereas a list of bug refs tells you "Here is a list of things to check you haven't broken". Unit tests are a nicer way of doing that, but not always possible (bugs only reproducible on customer systems etc).

Steve Lee said...

No I don't think it's good.
You want to read and understand the code in it's current state. If you want the history you can use the VC tools to get all the detail you want (assuming it's a good un and used well). The exception is if the code has some special 'twist' or subtlety to avoid a problem which should not be undone (I'm thinking code level problems not architectural).

I've worked on projects that did this and also got the VC to stamp a log at the top of the file and eventually decided this was all just noise that got in the way of reading the code

MartinG said...

No, I think it hurts readability, and worse, source should not depend upon anything external which might disappear in future.

Also, if there is something the developer needs to be careful about when changing that bit of code, everything they need to know should be commented in the code.

If the details in the bug are important for some reason, consider copying them into the commit message, and refer to the bug number in addition in the commit message if absolutely neccesary.

jcape said...

The only forseeable downside to putting bug numbers in your source code is that you need some kind of policy in place for your environment:

1.) Only use numbers from publicly-accessible trackers (no IDs from Company X's internal systems)

2.) Format them all the same way (so you can regex them out later when you abandon your own semi-internal Trac in favor of some big-project's bugzilla)

Ted Mielczarek said...

I find they're more useful for FIXME type comments, to have an actual bug on file for the thing that ought to be fixed, instead of just a nebulous statement that this ought to be fixed. :)

Alban said...

If you write a workaround in your program because of a bug in an external library, I think the reference of that bug must be in your source code because even when the bug is closed, your program can still be linked with a bogus version of the library.

If it is a bug in your own program, either the bug is not yet fixed so the reference is not in the source code, or the bug is fixed, and I prefer to have a good comment instead of a reference to a comment in a closed bug entry in bugzilla.

But if your program may communicate with previous version of your program that have the bug, I would put the bug reference in the code if you are writing a workaround to remain compatible.