How to remove large if-else-if chain

Question

Possible Duplicate:
Long list of if statements in Java

I was tasked to work with some code, and there is a giant if-else-if chain (100+ else-ifs) that checks Strings.

What are some good techniques to update this code as to where the if-else-if chain can be shrunken down to something much more manageable.

The chain looks something like this:

if(name.equals("abc")){
    do something
} else if(name.equals("xyz")){
    do something different
} else if(name.equals("mno")){
    do something different
} ......
.....
else{ 
   error
}

Solution 1

You can extract the code in each branch to a separate method, then turn the methods into implementations of a common base interface (let's call it Handler). After that, you can fill a Map<String, Handler> and just look up and execute the right handler for given string.

Unfortunately the implementation of 100+ subclasses for the interface requires quite a lot of boilerplate code, but currently there is no simpler way in Java to achieve this. Implementing the cases as elements of an Enum may help somewhat - here is an example. The ideal solution would be using closures / lambdas, but alas we have to wait till Java 8 for that...

Solution 2

Some options / ideas:

• Leave it as it is - it's not fundamentally broken, and is reasonably clear and simple to maintain
• Use a switch statement (if you are using Java 7) - not sure if this gains you much though
• Create a HashMap of String to FunctionObjects where the function objects implement the required behaviour as a method. Then your calling code is just: hashMap.get(name).doSomething();
• Break it into a heirarchy of function calls by sub-grouping the strings. You could do this by taking each letter in turn, so one branch handles all the names starting with 'a' etc.
• Refactor so that you don't pass the name as a String but instead pass a named object. Then you can just do namedObject.doSomething()

Solution 3

With Enums, you can have a method per instance.

public enum ActionEnum {
   ABC {
      @Override
      void doSomething() {
          System.out.println("Doing something for ABC");    
      }

   },
   XYZ {
      @Override
      void doSomething() {
         System.out.println("Doing something for XYZ"); 
      }
   };

   abstract void doSomething();
}

public class MyActionClass {

    public void myMethod(String name) {
        ActionEnum.valueOf("ABC").doSomething();
    }

}

It is still kinda messy (big enum with 100+ entries, even it all it does is dispatching), but may avoid the HashMap initialization code (100+ puts is also messy in my opinion).

And yet another option (for documentation purposes) would be reflection:

public interface Action {
    void doSomething();
}

public class ABCAction implements Action {
    @Override
    public void doSomething() {
        System.out.println("Doing something for ABC");      
    }
}

public class MyActionClass {

    void doSomethingWithReflection(String name) {
        try {
            Class<? extends Action> actionClass = Class.
                forName("actpck."+ name + "Action").asSubclass(Action.class);
            Action a = actionClass.newInstance();
            a.doSomething();
        } catch (Exception e) {
            // TODO Catch exceptions individually and do something useful. 
            e.printStackTrace();
        }
    }
}

Each approach has it's trade offs:

• HashMap = Fast + Kinda messy ("set-up" code with hundred of puts)
• Enum = Fast + Kinda messy 2 (huge file).
• Reflection = Slower + runtime error prone, but provides clean separation without resorting to clunky big HashMap.

Solution 4

Like Matt Ball said in his comment, you can use a command pattern. Define a collection of Runnable classes:

Runnable task1 = new Runnable() {
    public void run() { /* do something */ }
};
Runnable task2 = // etc.

Then you can use a map from your keys to runnables:

Map<String,Runnable> taskMap = new HashMap<String,Runnable>();
taskMap.put("abc", task1);
taskMap.put("xyz", task2);
// etc.

Finally, replace the if-else chain with:

Runnable task = taskMap.get(name);
if (task != null) {
    task.run();
} else {
    // default else action from your original chain
}

Solution 5

you can use the switch statement , but Switch statements with String cases have been implemented in Java SE 7

the best solution is to use the command pattern

View more solutions

Comments

[Alan Delimon]

The command pattern is the way to handle this for sure, see: stackoverflow.com/questions/1199646/…

[Mike Dunlavey]

What exactly is wrong with it as-is, other than seeming to be bothersome?

[Jorge]

it's not a bad idea but make a pattern of command i think that should be a better solution

[linuxuser27]

+1 That is my usual approach. Makes the code clean and easy to extend. When Java gets lambdas it will make it beautiful.

[Galya]

What about the fact that you need to keep many Handler instances in the Map that are probably not going to be used?

[Lukas Eder]

Did you measure this with JMH? It would be nice to know at what point the cost of dynamic dispatch is lower than the if-else branching.

[vivekmore]

I'm wondering if someone can provide an example of how to simplify this using lambdas (now that Java 8 is out)

[billoot]

@vivekmore Declare the map with something like Map<String, Handler> foo = new Map<>();, and populate the map with something like foo.put("bar", MyClass::doBar); assuming that class Handler is a functional interface.