Every once in awhile I run across a snippet of code which makes absolutely no sense to me. This snippet from SharpZipLib’s BZipOutputStream.cs file is one of those:
if (!(nHeap < (BZip2Constants.MAX_ALPHA_SIZE+2))) {
Panic();
}
Keep in mind that this source code is part of the examples to help newcomers understand how to use the library, so the odds are high that a reader will not understand the intricacies of the library.
OK, so a test is being done and things panic if the heap size is smaller than the MAX_ALPHA_SIZE plus two. Sure, that’s shiny and clear. But what the h3|| is MAX_ALPHA_SIZE, and why is two added to it? Maybe the help file will tell us something. Here’s what the documentation for that constant’s field says: “Backend constant.” Ah, OK, that’s perfectly clear.
I’m also happy that the developer took a moment to tell everyone why two was added to the test condition. Oh, wait. They didn’t. Maybe that value is added because the developer had two left feet? Maybe it’s the number of cheeseburgers they had for lunch.
I wonder if the developer who wrote that would themselves remember why the value is added six months down the road. Most likely not without some serious head scratching effort.
Do yourself and those who come after you a favor: help out a bit by describing why you’re altering a basic test condition. Better yet, note what constants and other field members are really supposed to do for a living.
Or maybe you just like to live by the principles of Unmaintainable Code.
I don’t mean to pick on the very nice SharpZipLib, this is just a pet peeve rant of mine and they happend to be in front of my nose today.
CORRECTION: This is from SharpZipLib’s source code, not the example files. Regardless, an explanation would have been really nice.
NOW PLAYING: The Stone Roses — Love Spreads. Man, what a great bunch of guitar and bass going on in this track. Replayed it twice already and it’s most likely gonna get hit again.
No comments:
Post a Comment