Hilarious Comment on Programming Stupidites Page

February 10th, 2017

I was reading a bunch of funny programming related stories on a page dedicated to programing stupidities, and came across this:

About four years ago, I was working on a project that, among other things, involved porting several million lines of code. While not technically real-time, the code needed to be reasonably fast. At one point, I found the following gem:

unsigned long reverse(unsigned long theWord)
{
    unsigned long result = 0;
    int i;
    for (i = 0; i < 32; ++i) {
        if (theWord & (unsigned long) pow(2.0, (double) i))
            result += (unsigned long) pow(2.0, (double) (31 – i));
    }
    return result;
}

Obviously, the purpose was to reverse the bits in a word. Naturally, I called all of my colleagues over to see this, and we all marvelled[sic] at how someone would think that a conversion to floating-point, a function call, and a conversion to integer could be faster than one shift operation. To say nothing of the possibility of rounding errors completely screwing up the, um, algorithm.

Not wanting to leave an exercise for the reader, here’s the replacement:

unsigned long reverse(unsigned long theWord)
{
    unsigned long result = 0;
    int i;
    for (i = 0; i < 32; ++i) {
        if (theWord & (1 << i))
            result += 1 << (31 – i);
    }
    return result;
}

I just wanted to point out the irony of it: I could say that “I marvelled[sic] at how someone would think that using an “if” test in this code is at all optimal.” But I won’t do that since my replacement might not be the pinnacle of performance either. I’m just pointing out that it’s funny to see someone write unoptimal code while condemning someone else’s unoptimal code. Here’s my better replacement, which may not be the best but is better than that person’s best:

unsigned long reverse(unsigned long theWord)
{
    unsigned long result = 0;
    int i;
    for (i = 31; i >= 0; ––i) {
         result |= ( theWord & 1) << i;
        theWord >>= 1;
     }
    return result;
}

There is no “if” test in this. There seems to be one less math operation too, but I’m not sure if the loop counter is optimal. I could have left the loop as “for( i=0…” and then written slightly more complex code but I’m not sure if the more complex shifting counteracts a more efficient loop counter.

The lesson? Humility is important. Don’t condemn someone’s code unless you are 100% sure that yours is not also condemnable. Or at least admit that you are not sure yours is the best either.

Also, long values could be more than 32 bits in these modern times, so a bit count check at the beginning of the function, or a comment about assuming that “long” is 32 bits, would be appropriate in there.

Leave a Reply

You must be logged in to post a comment.