Refactor if statement to use appropriate pattern

Question

I have an enum with some states in it:

enum State
{
    A,
    B,
    C,
    D
}

and an object that has a corresponding state:

class MyObject
{
    State state;
}

I need to write an algorithm that takes two MyObject instances and does something depending on the particular states of those instances:

void doWork(MyObject o1, MyObject o2)
{
     if (o1.state == A && o2.state == A)
     {
          // do something
     }
     else if (o1.state == A && o2.state == B)
     {}
     // etc for all combinations...

}

Obviously this approach has many problems and I would like to change it to ideally get rid of the if/else statement.

Is there any pattern for such a requirement?

Thanks

@org.life.java: nonsense - using == is completely safe with enums. — Michael Borgwardt, Nov 12 '10 at 19:40
@Michael Borgwardt I apologize,deleted comment also for the same scenario `==` is more preferable , learned from this http://stackoverflow.com/questions/1750435/comparing-java-enum-members-or-equals — jmj, Nov 12 '10 at 19:53

stakx - no longer contributing · Accepted Answer · 2010-11-12T19:39:37.543

What you could do, although I'm not sure it would be that much better, is some kind of matrix of all possible combinations of two state values; you could then use o1.state and o2.state as indexes into that matrix.

You could store different things in that matrix:

a unique value that you can use as the discriminating value for a switch block which would replace your if .. else if .. else blocks -- not much of an improvement, really.

Or your matrix could contain...

command objects. (Look up the Command Pattern.)

If you really want to get rid of the if statements, that second option might be the better one; do take note, however, that your code will then no longer be close together in one location, as would be the case with if/switch blocks, but spread over several different command objects/classes.

// forgive my syntax errors etc., my Java has definitely gone a little rusty!

interface WorkCommand {
    public abstract void run(MyObject o1, MyObject o2);
}

...

Map<Pair<State,State>, WorkCommand> commands;
// ^ pseudo-code type for your command look-up map; type Pair<X,Y> doesn't exist,
//   so replace this with something sensible!

void doWork(MyObject o1, MyObject o2)
{
    WorkCommand worker = commands.get(new Pair<State,State>(o1, o2));
    worker.run(o1, o2);
}

John Vint · Answer 2 · 2010-11-12T19:50:48.737

One way you can structure this, is you can have an abstract method in your enum which each element would implement:

enum State
{
    A{
      public void void doSomeWork(State state){
        switch(state){
           case A:
           case B:
           ...
        }
      }
    },
    B,
    C,
    D

    abstract void doSomeWork(State state);
}

Then your method can look like

void doWork(MyObject o1, MyObject o2){
   o1.state.doSomeWork(o2.state);
}

score 2 · Answer 3 · answered Nov 12 '10 at 20:07

Yes, it is called the... state pattern. The important thing is to only have one state for which to define behavior, i.e. you may need to combine your object1.state and object2.state into a meta-state. Register this meta-state with a statecontext so that when Myobject.state changes the meta-state is updated.

interface MyObjectStates {
  void doWork(MyObject o1, MyObject o2);
}

class MyObjectStatesAA implements MyObjectStates {
  void doWork(MyObject o1, MyObject o2) {
    // do dowork for both states A
  }

class MyObjectStatesBB implements MyObjectStates {
  void doWork(MyObject o1, MyObject o2) {
    // do dowork for both states B
  }

// etc

You then need to hold one MyObjectStates object in a statecontext and update it when a MyObject.state is changed. You may even be able to remove the state enum alltogether. If this approach sounds interesting to you give me a note and I elaborate if you like.

The state pattern has the advantage that you don't need to save and read back an enum and chose a different codepath accordingly, but instead you provide separate code with every state you want to handle differently.

This is the only solution that really gets rid of conditional statements -- switch/case is still an ugly conditional statement (unless you're writing parsers). — Ted M. Young, Feb 07 '12 at 00:27

score 1 · Answer 4 · answered Nov 12 '10 at 19:40

I 'd done this probably , its at least more readable

void doWork(MyObject o1, MyObject o2) {
    switch (o1.state) {
        case A: {
            switch (o2.state) {
                case A: {

                    break;
                }
                case B: {

                    break;
                }

            }
            break;
        }
    }
}

score 0 · Answer 5 · answered Nov 12 '10 at 19:48

using only two state combinations, a nested switch is probably the quickest to implement and comprehend:

switch (o1.state) {
 case X: switch(o2.state) { }
 //..etc
}

If the case order is irrelevant for certain combinations, you may be able to swap o1 and o2 in those cases, then drop them into the switch (and avoid duplicate code). Also, for all the cases combinations that have the same behavior, you can take advantage of the fall through behavior.

Finally, implementing it this way may make what's actually going on with those combinations a little more obvious, such that you might be able to implement a much smarter approach.

score 0 · Answer 6 · answered Nov 12 '10 at 20:10

The OO solution to your problem is the state pattern. It involves encapsulating state-alternating conditionals with a state object. You may find more information about the pattern here and here. By the way, i would STRONGLY suggest buying this book if you don't already own it.

Cheers!

Refactor if statement to use appropriate pattern

6 Answers6