CodeSOD: Black Letters

Johannes started debugging an application, and decided he needed to “share his pain”.

Here, we’re presented with a simple problem: convert a number in the range [0-25] to a letter [A-Z]. Many people would solve this with an array of letters as a lookup table. If they’re clever, they’d leverage the character encoding and do some arithmetic.

Or, they could just have a gigantic switch:

private function getLetter($value, $type = 0){
$data = ”;

if($type == 0){
switch($value){
case 0: $data = ‘A’; break;
case 1: $data = ‘B’; break;
case 2: $data = ‘C’; break;
case 3: $data = ‘D’; break;
case 4: $data = ‘E’; break;
case 5: $data = ‘F’; break;
case 6: $data = ‘G’; break;
case 7: $data = ‘H’; break;
case 8: $data = ‘I’; break;
case 9: $data = ‘J’; break;
case 10: $data = ‘K’; break;
case 11: $data = ‘L’; break;
case 12: $data = ‘M’; break;
case 13: $data = ‘N’; break;
case 14: $data = ‘O’; break;
case 15: $data = ‘P’; break;
case 16: $data = ‘Q’; break;
case 17: $data = ‘R’; break;
case 18: $data = ‘S’; break;
case 19: $data = ‘T’; break;
case 20: $data = ‘U’; break;
case 21: $data = ‘V’; break;
case 22: $data = ‘W’; break;
case 23: $data = ‘X’; break;
case 24: $data = ‘Y’; break;
case 25: $data = ‘Z’; break;
default: $data = ‘XYZ’; break;
}
}else{

}

return $data;
}

Honestly, though, the switch isn’t the worst part of this. I’d argue the gigantic switch is bad, but not a WTF. No, there are two worse things.

First, when the number is outside of the range, it returns ‘XYZ’. That’s not a letter. That’s a multi-character string. I’d rather get a null, or a zero, or a thrown exception. This is weird, unexpected, and potentially breaks code that foolishly assumes getLetter returns a single character string.

Second: what the hell is type? Here we have a parameter that just… makes the function not work? Were they hoping to one day make this multilingual (and were they planning to make switches for every new language)? Did they just forget why they accepted the parameter and decided this was the best they could do?

As it turns out, many functions accept a type parameter. Some do something with it. Some don’t. No one knows why.

Source: Read More