22

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
}
2
  • 1
    The command pattern is the way to handle this for sure, see: stackoverflow.com/questions/1199646/… Commented Sep 8, 2011 at 14:50
  • What exactly is wrong with it as-is, other than seeming to be bothersome? Commented Sep 8, 2011 at 14:56

6 Answers 6

18

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...

Sign up to request clarification or add additional context in comments.

5 Comments

+1 That is my usual approach. Makes the code clean and easy to extend. When Java gets lambdas it will make it beautiful.
What about the fact that you need to keep many Handler instances in the Map that are probably not going to be used?
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.
I'm wondering if someone can provide an example of how to simplify this using lambdas (now that Java 8 is out)
@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.
9

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()

Comments

8

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
}

Comments

8

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.

Comments

2

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

1 Comment

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

This is a popular Arrow Anti-Pattern and Jeff discusses some approaches to handle this very nicely in his post here.

1 Comment

This is NOT the Arrow anti-pattern.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.