CodeSOD: Maximally Zero

Today’s anonymous submitter found some Java code which finds the largest value in a quartet of floats. Now, the code is quite old, so it actually predates varargs in Java. That doesn’t excuse any of what you’re about to see.

public float CalculateMaximumValue(float a, float b, float c, float d) {
int i = 0;
float[] arr = new float[] { 0, 0, 0, 0 };
float gtval = 0;

for (i = 0; i < 4; i++) {
arr[i] = 0;
}
arr[0] = a;
arr[1] = b;
arr[2] = c;
arr[3] = d;
gtval = arr[0];
for (i = 0; i < 4; i++) {
if (arr[i] > gtval) {
gtval = arr[i];
}

}

return gtval;

}

The best thing I can say about this is that they didn’t use some tortured expansion of every possible comparison:

if (a > b && a > c && a > d) return a;
if (b > a && b > c && b > d) return b;
…

Honestly, that would be awful, but I’d prefer it. This just makes my eyes sting when I look at it.

But let’s trace through it, because each step is dumb.

We start by creating an empty array, where every value is initialized to zero. This isn’t necessary, as that’s what Java does by default. But then, we loop across the array to set things to zero one more time, just to be sure.

Once we’re convinced every value is definitely zero, we replace those zeroes with the real values. Then we can loop across the array and find the largest value with straightforward comparisons.

This code is, in some ways, the worst kind of code. It’s bad, but not so bad as it’s ever going to cause real, serious problems. No one is going to see any bugs or inefficiencies coming from this method. It’s just an ugly mess that’s going to sit there in that codebase until the entire thing gets junked, someday. It’s just an irritant that never rises to the level of frustration which drives action.

Source: Read More