Refactoring if/else logic

I have a java class with a thousand line method of if/else logic like this:

if (userType == "admin") {
     if (age > 12) {
          if (location == "USA") {
               // do stuff
          } else if (location == "Mexico") {
               // do something slightly different than the US case
          }
     } else if (age < 12 && age > 4) {
          if (location == "USA") {
               // do something slightly different than the age > 12 US case
          } else if (location == "Mexico") {
               // do something slightly different
          }
     }
 } else if (userType == "student") {
     if (age > 12) {
          if (location == "USA") {
               // do stuff
          } else if (location == "Mexico") {
               // do something slightly different than the US case
          }
     } else if (age < 12 && age > 4) {
          if (location == "USA") {
               // do something slightly different than the age > 12 US case
          } else if (location == "Mexico") {
               // do something slightly different
          }
     }

How should I refactor this into something more managable?


Solution 1:

You should use Strategies, possibly implemented within an enum, e.g.:

enum UserType {
  ADMIN() {
    public void doStuff() {
      // do stuff the Admin way
    }
  },
  STUDENT {
    public void doStuff() {
      // do stuff the Student way
    }
  };

  public abstract void doStuff();
}

As the code structure within each outermost if branch in your code looks pretty much the same, in the next step of refactoring you might want to factor out that duplication using template methods. Alternatively, you might turn Location (and possibly Age) into a strategy as well.

Update: in Java4, you can implement a typesafe enum by hand, and use plain old subclassing to implement the different strategies.

Solution 2:

The first thing I would do with this code is create the types Admin and Student, both of which inherit from the base type User. These classes should have a doStuff() method where you hide the rest of this logic.

As a rule of thumb, any time you catch yourself switching on type, you can use polymorphism instead.

Solution 3:

Thousands? Maybe a rules engine is what you need. Drools could be a viable alternative.

Or a Command pattern that encapsulates all the "do something slightly different" logic for each case. Store each Command in a Map with the concatentation of age, location, and other factors as the key. Lookup the Command, execute it, and you're done. Nice and clean.

The Map can be stored as configuration and read in on start up. You can add new logic by adding new classes and reconfiguring.