开发者

How to remove large if-else-if chain [duplicate]

This question already has answers here: Closed 11 years ago.

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
}


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


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


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.


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
}


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


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

0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜