Bonehead Programming

Why do some programmers write stupid code? Simple. Nobody every told them it was stupid, explained why, or told them a better way of doing it.

Each of the tasks below shows one or more "bonehead" solutions (in Java), an explanation of why it's a stupid solution, and then a better solution. Study and learn, grasshopper. Don't be a bonehead.

Task 1: Calculate x².

Bonehead Solution: square = Math.pow(x,2);

What's wrong with it?

It's difficult to read. The improved solution below is much easier to comprehend.
For those concerned about efficiency, the pow() function is slow.

Better Solution: square = x * x;

Similarly, Math.sqrt(x) is better thanMath.pow(x, 0.5);

Task 2: Converting an alphabetic character between A and Z to an integer from 0 to 25.

Bonehead Solution A:

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

Bonehead Solution B

Similar to solution A except uses multiple IF statements.

What's wrong with it?

It's easy to make mistakes. As simple as it may seem, I've seen many solutions like this with incorrectly typed letters or numbers producing incorrect results.
It's not easy to modify. Say the problem changes so that the range of return values becomes 1 - 26. That requires changing every return statement in the code.
It's FAT. It requires 28 lines to code what can be performed in one (see solution below).

Better Solution

return (letter - 'A');
(Note: this only works for a character set like ASCII that has consecutive characters).

Task 3: Determine if a character is a digit in the range 0 to 9.

Bonehead Solution A:

return (letter == '0') || (letter == '1') || (letter == '2') || (letter == '3') ||
(letter == '4') || (letter == '5') ||(letter == '6') ||(letter == '7') ||
(letter == '8') ||(letter == '9') ;

Bonehead Solution B:

return (letter >= '0') && (letter <= '9');

What's wrong with it?

It's easy to make mistakes by mistyping the boolean operator or the relational operators.
It fails to reuse an existing function in the language which performs this task.

Better Solution

return Character.isDigit(letter);

Task 4: make a decision based on the results of some boolean function (for example, isEmpty()).

Bonehead Solution

if (mystack.isEmpty() == true) dosomething;

What's wrong with it?

A boolean function returns a boolean value, so it doesn't need to be compared to true in order to produce a boolean expression.
It doesn't read well: the relational operator interferes with a smooth reading of the statement.

Better Solution

if (mystack.isEmpty()) dosomething;

Task 5: return a boolean value based on some boolean expression

Bonehead Solution

if (size > 0)
return true;
else
return false;

What's wrong with it?

It takes four lines to accomplish what can be coded in one.
It's redundant. The expression (size > 0) is a boolean expression that will be evaluated to true or false so it's unnecessary to put it into a conditional statement.

Better Solution

return size > 0;

Task 6: initialize an array.

Bonehead Solution

private int[] scores = new int[10];

scores[0] = 5;
scores[1] = 3;
scores[2] = 7;
scores[3] = 8;
scores[4] = 2;
scores[5] = 3;
scores[5] = 5;
scores[7] = 6;
scores[8] = 9;
scores[9] = 3;

What's wrong with it?

It's easy to make mistakes. (Can you find the error in the example above?)
It's FAT.

Better Solution

private int[] scores = {5, 3, 7, 8, 2, 3, 5, 6, 9, 3};

Task 7: Assign a value to a variable based on the value of a second variable.

Bonehead Solution

if (turn == 1) color = 1;
elseif (turn == 2) color = 2;
elseif (turn == 3) color = 3;
elseif (turn == 4) color = 4;
elseif (turn == 5) color = 5;
elseif (turn == 6) color = 6;
elseif (turn == 7) color = 7;
elseif (turn == 8) color = 8;
else color = 9;

What's wrong with it?

I have no idea why someone would go to all this trouble to obfuscate a trivial operation, but I've seen it done.

Better Solution

color = turn;

Task 8: Convert a string representation of a number into a Java int.

Bonehead Solution

public static int stringToInt(String input)
{
    int result = 0;
    int sLen = input.length();
    for (int i = 0; i < sLen; i++)
    {
        result = 10 * result + ((int) (input.charAt(i)) - 48);
    }
    return result;
}

What's wrong with it?

Java already has a function to do this. This is a clear symptom of ignorance of the language. This is only one of many examples of this kind of boneheadedness. Others functions already available in Java for which ignorant programmers write their own code include:

converting a vector to an array - copyInto()
sorting a collection - sort()
finding the position of a character in a string - indexOf()
breaking a string into tokens - StringTokenizer or split()

Better Solution

result = Integer.parseInt(input);

Task 9 Advance the turn among players in a multiplayer game (or any situation that needs to maintain a counter that "rolls over").

Bonehead Solution

if (player ==1)
{
if (numberOfPlayers >= 2) player = 2;
else player = 1
}
else if (player == 2)
{
if (numberOfPlayers >= 3) player = 3;
else player = 1
}
.
.
. (repeats five more times!)
.
else if (player == 8)
{
if (numberOfPlayers >= 9) player = 9;
else player = 1
}

What's wrong with it?

The code is FAT and not easy to read.
The code is not easily extended for more than 9 players.
It uses redundant logic to perform a simple mathematical function that is provided by a primitive operator in the language, "remainder".

Better Solution:
        player = player + 1;
        if (player > numberOfPlayers)
            player = 1;

Best Solution:
player = (player % numberOfPlayers) + 1;

Home