Refactoring multiple if-else conditional conditions in a method

Question

I am reorganizing my existing code. It really works fine, but it is a bit cluttered with a few if-else conditionals that check the value of one variable and change the value of the second variable to an updated value taken from a fixed enumeration structure.

else if (var1 == 'valueX') { if (var2 == MyEnum.A) var2 = MyEnum.B; else if (var2 == MyEnum.B) var2 = MyEnum.C; else if (var2 == MyEnum.C) var2 = MyEnum.D; else if (var2 == MyEnum.D) var2 = MyEnum.A; } else if (....) { ..similar block of conditionals } 

I'm a little confused about what is the best way to refactor and clean up this code. Would you suggest using a switch? Or something more elegant?

Thanks in advance!

Answer 1

At least with J2SE 1.5 onward, you can specify additional enumeration attributes. This means that you could replace the entire whole if-else line with something that looks like

 var2 = var1.getNextInSequence(); 

Now, in this case, it looks like you want the attribute to be a link to another enumeration that adds some wrinkles, for example, you cannot forward reference enumerations when they are initialized, but a solution for you in this way may be workable.

If the attributes are not other instances of the same enumeration, this will work:

 public enum Animal { FOX(4), CHICKEN(2), WORM(0); private int countLegs; Animal(int n) { countLegs = n; } public int getLegCount() { return countLegs; } // .. more getters setters etc } 

But when the enumeration is self-referential, you have to be careful with the order in which your instances are declared. Ie this will have some problems:

 public enum Animal { FOX(4, CHICKEN), // 'CHICKEN' doesn't exist yet WORM(0, null), CHICKEN(2, WORM); // this actually will compile private int countLegs; private Animal eatsWhat; Animal(int n, Animal dinner) { countLegs = n; eatsWhat = dinner; } public int getLegCount() { return countLegs; } // .. getters, setters, etc } 

So, if you need a circular set of links among the enumerations, you will have to work on something else, but if not, you can use this technique, although you may have to order enum instances just to make it work.

Answer 2

The classic answer to refactoring conditions . Replace conditional with polymorphism . In this case, if each of MyEnum knew who its successor was, you can simply say (in the case of "valuex": var2 = var2.successor. For var1, if it could be an object that implemented an interface that knew how to handle no matter what you do inside the loop, and each implementing class knew exactly what it should do ... Well, you would.

Update:

But in the test case, the function of the dandy-small successor:

 public class EnumTest extends TestCase { private enum X { A, B, C; public X successor() { return values()[(ordinal() + 1) % values().length]; } }; public void testSuccessor() throws Exception { assertEquals(XB, XAsuccessor()); assertEquals(XC, XBsuccessor()); assertEquals(XA, XCsuccessor()); } } 

Answer 3

You can use a simple map:

 enum MyEnum { A, B, C }; Map<MyEnum, MyEnum> VALUE_X = new HashMap<MyEnum, MyEnum>() {{ put(MyEnum.A, MyEnum.B); put(MyEnum.B, MyEnum.C); ... }}; // define another kind of ordering Map<MyEnum, MyEnum> VALUE_Y = new HashMap<MyEnum, MyEnum>() {{ put(MyEnum.A, MyEnum.D); put(MyEnum.B, MyEnum.A); ... }}; 

Thus, the logic of the following var2 value is not hardcoded in the enumeration itself and may depend on the context (i.e., the value of var1 ):

 if ("valueX".equals(var1)) { // use equals() instead of == for Strings var2 = VALUE_X.get(var2); } else if ("valueY".equals(var1)) { var2 = VALUE_Y.get(var2); }