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
}
Community
  • 1
  • 1
user906153
  • 1,218
  • 8
  • 30
  • 43

6 Answers6

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

Community
  • 1
  • 1
Péter Török
  • 114,404
  • 31
  • 268
  • 329
  • +1 That is my usual approach. Makes the code clean and easy to extend. When Java gets lambdas it will make it beautiful. – linuxuser27 Sep 08 '11 at 14:50
  • What about the fact that you need to keep many Handler instances in the Map that are probably not going to be used? – Galya Jan 08 '16 at 08:40
  • Did you measure this with JMH? It would be nice to know at what point the cost of [dynamic dispatch](http://shipilev.net/blog/2015/black-magic-method-dispatch/) is lower than the if-else branching. – Lukas Eder Jan 10 '16 at 18:57
  • I'm wondering if someone can provide an example of how to simplify this using lambdas (now that Java 8 is out) – vivekmore Jan 17 '17 at 15:42
  • 1
    @vivekmore Declare the map with something like `Map foo = new Map<>();`, and populate the map with something like `foo.put("bar", MyClass::doBar);` assuming that class `Handler` is a functional interface. – billoot Feb 02 '17 at 16:13
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()
mikera
  • 105,238
  • 25
  • 256
  • 415
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.
Anthony Accioly
  • 21,918
  • 9
  • 70
  • 118
7

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
}
Ted Hopp
  • 232,168
  • 48
  • 399
  • 521
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

confucius
  • 13,127
  • 10
  • 47
  • 66
  • 1
    it's not a bad idea but make a pattern of command i think that should be a better solution – Jorge Sep 08 '11 at 14:48
-1

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

Jugal Shah
  • 3,621
  • 1
  • 24
  • 35