Legacy Best Practices

This disturbing code snippet comes from the Daily WTF.

public boolean isBooleanFalse(boolean value) {
   boolean response = false;
   if (value == true) {
       response = false;
   } else {
       response = true;
   }
   return response;
}


It’s difficult to imagine what the author was thinking. I’d like to believe it’s a contrived example, but I’ve seen similar code. This method could easily be reduced to public boolean isBooleanFalse(boolean value) { return !value; }. Better yet, calls to the could be replaced with !value.

Why do programmers write code like this? My first thought was blame the programmer for not having learned how to use the ! operator. Or maybe it’s Martin Folwer’s fault for publishing the Replace Temp with Query refactor that recommends trivial methods like this. However, I think part of the responsibility falls on antiquated best practices and coding standards, like those left over from K&R C.

An Assistant Professor of Psychology at Stanford, wrote, “one’s native language appears to exert a strong influence over how one thinks.”[pdf] It’s reasonable to extend this conclusion to programming languages. The style we use and the coding standards we follow affect how we solve problems.

Consider how reversing the following guidelines push the suspect code snippet closer to the optimal solution.

Declare variables at the top of methods
By habit, some C programmers declare and initialize variables before the first instruction. One of many side effects is that temporary variables seem to be created early in the processes, leading to variables that are not necessary.
Single exit point
This single exit point practice states that there should be a single return. The optimal version of this method requires only one return. However, adding a return to each case of the if-else would remove the temporary variable, leading the programmer one step closer. There are coding standards today that still forbid this.
No ternary operators
Some organizations hire developers that can’t grok (expression) ? true : false;, so they forbid it’s use. The ? operator is abused, but so are switch statements, inheritance, reflection, and function pointers.
Not using Booleans like Booleans
C added bool over a decade ago, though some developers are still waiting to use them. Since these guys are typically using integers, the correct expression is if (MyBool == OVERLOADED_TRUE), rather than the more concise if (MyBool). These developers even bring this style into Java and C#, without considering why it existed.



These apply less today than they did on mainframes in 1978. Not only do they add noise, they also hinder our problem solving abilities. I’m not advocating the use every good tool or pattern that comes along, but we need to be more skeptical of long standing conventions.

Tags: