Conditional Spaghetti

 This article was originally posted on JRoller on June 24, 2004

Often, having a lot of if/else structures one after the other in a code means that something is wrong with the design. It's what Kent Beck would call a Bad Smell. And usually, you can remove the conditions by using polymorphism. However, it is not always possible, or using inheritance would complicate the design too much.

Here, I want to speak about a certain kind of conditional spaghetti, where the list of if/else is quite long and the body is the same for each branch. This case can be viewed as a conversion between 2 values.

Here is a code that I've seen in one of the programs I had to review. It decides what icon to use for a given object type. There were something like 80 conditional branches.

if (getType().compareTo("PSTNGateway") == 0)     node.setIcon(new ImageIcon(       RCMGR.getImage("Image.Gateway")));   else if (getType().compareTo("PSTNNode") == 0)     node.setIcon(new ImageIcon(       RCMGR.getImage("Image. Node")));   else ...

My solution for this kind of code is to use a static HashMap for converting the object's name to its icon. Here is the HashMap declaration:

private static HashMap nodeTypes = new HashMap(50);   static {     nodeTypes.put("PSTNGateway", "Image.Gateway");     nodeTypes.put("PSTNNode", "Image.Node");     ...   }

Then, the 160 lines of conditional code becomes a simple two lines of code:

node.setIcon(new ImageIcon(RCMGR.getImage((String)     nodeTypes.get(getType()))));

Not only it is easier to read and maintain, but the 40 String comparisons on average become a hash code calculation and a couple of comparisons. The same technique can be applied to a Factory. From the same program, the code that was creating the nodes according to some parameter String looked like this:

if (cPar.getType().compareTo("BSC") == 0)     newNode = new TDBSC(cPar.getName());     else if (cPar.getType().compareTo("RNC") == 0)     newNode = new TDRNC(cPar.getName());     else ...

Now using the HashMap to convert between the node's name and the corresponding class:

private static HashMap nodeTypes = new HashMap(150);   static {     nodeTypes.put("BSC", TDBSC.class);     nodeTypes.put("RNC", TDRNC.class); ...   }

Class nodeClass = (Class)nodeTypes.get(cPar.getType());   Constructor init = nodeClass.getConstructor(new Class[] {String.class});   newNode = (TDTreeNode)init.newInstance(new Object[] {cPar.getName()} );

As a final note, I want to mention that the HashMap can be totally avoided by using naming convention. Then, the node class for example can be inferred with a simple formula:

Class nodeClass = Class.forName(PACKAGE_NAME + CLASS_PREFIX + nodeType);

This article was written in Java 1.4. Now, of course, using generics, you can avoid all those ugly castings. I used this pattern several times following this article. The gain in number of code lines is big, but it comes also with a notable gain in performance.