0

I have an abstract class X and some classes who extend this class, call them A, B and C.

In some other class Y I have a few methodcalls that depend on the type of the class. The if-else statement looks like this:

public class Y implements InterfaceY {

    public Y(){
    }

    public String doStuff (X x, Boolean flag) {
    String s = "";
    if (x instanceof A) {
        doStuff((A) x));
    } else if (x instanceof B) {
        doStuff((B) x));
    } else if (x instanceof C) {
        doStuff((C) x, flag); 
    } else {
        throw new Exeption();
    }
    return s;

    private String doStuff(A a) {
        return "";
    }

    private String doStuff(B b) {
        return "";
    }

    private String doStuff(C c, Boolean flag) {
        return "";
    }
}

Note that all methods have the same name (doStuff()) but depending on the class (and sometimes flag) call a different method implementation of that method. Of course this looks horrible and gets immensely complicated once the classed that are extended from X increase.

Is there any way that I can somehow create an intermediate Interface that (or something else) that takes care of the majority (or all) of the if-else statements?

Tim
  • 2,000
  • 4
  • 27
  • 45
  • In your if's, should `X` be `x`? – jonhopkins Sep 19 '13 at 14:44
  • @jonhopkins, yes indeed. I will fix it. – Tim Sep 19 '13 at 14:45
  • 1
    Usually, in an Object Oriented context, if you use too much of `instanceof` your design is probably wrong. Of course that is why you are posting here :) – Cruncher Sep 19 '13 at 14:52
  • 1
    I think that before we can give a correct answer, we need to know: (1) Are you able to add methods to `X`? (2) Are you able to add methods to `A`, `B`, and `C`? If they're all part of your own program, the answers to both questions should be yes; but sometimes we have to face these problems with library classes we can't modify. – ajb Sep 19 '13 at 15:06
  • @ajb, I am allowed to change any class I'd like. But since this is merely a very small part of a huge product changing something in an abstract/super/interface class would mean refactoring a whole bunch of code. That is why the best solution for me would be some intermediate class. – Tim Sep 20 '13 at 06:52

6 Answers6

1

First take these methods out of here, and put them in the A, B and C class respectively, implementing the X interface.

private String doStuff(A a) {
    return "";
}

private String doStuff(B b) {
    return "";
}

private String doStuff(C c, Boolean flag) {
    return "";
}

Then:

if (x instanceof A) {
    doStuff((A) x));
} else if (x instanceof B) {
    doStuff((B) x));
} else if (x instanceof C) {
    doStuff((C) x, flag); 

can just be x.doStuff(); (you don't even have to pass the A, B, C because that will be this inside the method. The flag you'll have to mess around with depending more specifically on your code. for example, the other 2 doStuff methods could accept the flag as well, but just ignore it)

Cruncher
  • 7,641
  • 1
  • 31
  • 65
  • You can't do `x.doStuff()` unless you also define `doStuff` in `X`. And the method **must** take a `flag` parameter in all the classes (`X`, `A`, `B`, `C`) because the profiles have to be the same in order for the polymorphism to work. Finally, while I think this is the "correct" solution, we don't know whether the OP has the ability to modify `X`. Sometimes you face problems like this with a class in somebody else's library. – ajb Sep 19 '13 at 15:05
  • @ajb Yes, obviously I was talking about x as an interface having doStuff. I also addressed taking the flag parameter. Considering that OP is (more or less) looking for a refactor of the code, I assumed that everything was up for grabs. – Cruncher Sep 19 '13 at 15:08
  • sorry, when you said they "could" accept a flag I interpreted that to mean it was optional. It's not. – ajb Sep 19 '13 at 15:10
  • @ajb by "could", I meant that's a potential solution to the problem. The only one I came up with immediately, but there may be others. Specifically to this problem you may be able to infer something about flags. I did not mean that there is no problem, and that you may do this if you like. – Cruncher Sep 19 '13 at 15:15
  • @Cruncher I will give this solution a try, it seems like a good idea. But since my example is merely a small part of a huge product this might lead to major refactoring, but it also might not; I will have to took in to this. – Tim Sep 20 '13 at 06:56
0

APPROACH 1

Use the state pattern. It takes care of your problem and eliminates the ifs and elses.

Here is the java example.

The state pattern delegates the methods calls to objects that implement the same interface but with different behaviour.

State pattern example:

public class StatePatternExample {
    public static void main(String[] args) {
        Girlfriend anna = new Girlfriend();
                            // OUTPUT
        anna.kiss();        // *happy*
        anna.greet();       // Hey, honey!
        anna.provoke();     // :@
        anna.greet();       // Leave me alone!
        anna.kiss();        // ...
        anna.greet();       // Hey, honey!
    }
}

interface GirlfriendInteraction extends GirlfriendMood {
    public void changeMood(GirlfriendMood mood);
}

class Girlfriend implements GirlfriendInteraction {
    private GirlfriendMood mood = new Normal(this);

    public void provoke() {
        mood.provoke();
    }
    public void kiss() {
        mood.kiss();
    }
    public void greet() {
        mood.greet();
    }
    public void changeMood(GirlfriendMood mood) {
        this.mood = mood;
    }
}

interface GirlfriendMood {
    public void provoke();
    public void kiss();
    public void greet();
}

class Angry implements GirlfriendMood {
    private final GirlfriendInteraction context;

    Angry(GirlfriendInteraction context) { // more parameters, flags, etc. possible
        this.context = context;
    }
    public void provoke() {
        System.out.println("I hate you!");
    }
    public void kiss() {
        System.out.println("...");
        context.changeMood(new Normal(context));
    }
    public void greet() {
        System.out.println("Leave me alone!");
    }
}

class Normal implements GirlfriendMood {
    private final GirlfriendInteraction context;

    Normal(GirlfriendInteraction context) {
        this.context = context;
    }

    public void provoke() {
        System.out.println(":@");
        context.changeMood(new Angry(context));
    }

    public void kiss() {
        System.out.println("*happy*");
    }

    public void greet() {
        System.out.println("Hey, honey!");
    }
}

As you can see, the class Girlfriend has no ifs and elses. It looks pretty clean.

The class Girlfriend corresponds to your abstract class X, the classes Normal and Angry correspond to A, B and C.

Your class Y then directly delegates to X without checking any cases.

APPROACH 2

Use the command pattern. You could then hand over a command object to Ys doStuff() method and just execute it.

mike
  • 4,929
  • 4
  • 40
  • 80
  • Please clarify in what way the `state` pattern would be used to solve this issue. – John B Sep 19 '13 at 14:48
  • don't see how this would solve the question. You have described the state pattern but not how it would be used solve this problem. – John B Sep 19 '13 at 15:21
  • `Visitor` would be more like it. This seems to be the wrong approach. – Dirk Sep 19 '13 at 15:26
  • @Dirk He doesn't want to outsource an expensive operation or code per se. He wants do react differently depending on the state (in this case state of the method). Therefore he should incorporate the state pattern into his design. – mike Sep 19 '13 at 15:29
  • @JohnB Drew some parallels, that make clear why my answer helps OP to eliminate `if`s and `else`s - that was his question. – mike Sep 19 '13 at 15:37
  • @mike. I beg to differ. The dispatch happens on the `x` parameter value, which is not part of the state of a `Y` but comes as a parameter value. But maybe I'm dense and don't get your point. Could you rewrite your example so, that it matches the OP's one more closely? – Dirk Sep 19 '13 at 15:51
  • @Dirk I can't give a full example, since I don't know who provides the parameter `X` in `Y`s `doStuff()` (...really nice names btw...) and how it is provided. – mike Sep 19 '13 at 15:55
  • I will state that I do not believe this matches the `state` pattern. Nothing is changing state here. Simply the class is taking a different action based on different input. It does not change state (maintain some information or behavior) based on the input. – John B Sep 19 '13 at 16:00
  • @JohnB Because we don't know where and how the type (resp. the concrete subtype) of the input paramter `X` is determined. I would be happy if OP did provide more information. – mike Sep 19 '13 at 16:04
0

What about implementing a Handler interface then map it by supported type:

public interface Handler<T extends X>{
     Class<T> supportedClass;

     void doStuff(T value, Object...args);
}


public class Y implements InterfaceY {

       private Map<Class<?>, Handler<?>> handlers;

      public Y(List<Handler<?>> handlers){
          // populate map
      }


      public void process(X value){
           handler.get(value.getClass).doStuff(X, ...);
           // you would have to figure out how to determine when other values are needed
      }
}
John B
  • 32,493
  • 6
  • 77
  • 98
0

What about some double dispatch?

class X {
    public String letYdoStuff(Y y, Boolean flag) {
        return y.doStuff(this, flag);
    }

    public static void main(String [] args) {
        //X x = new A();
        X x = new B();
        Y y = new Y();

        y.doStuff(x, false);
    }

    public X getThis() {
        return this;
    }
}

class A extends X {
    public String letYdoStuff(Y y, Boolean flag) {
        return y.doStuff(this, flag);
    }
}
class B extends X {
    public String letYdoStuff(Y y, Boolean flag) {
        return y.doStuff(this, flag);
    }
}
class C extends X {
    public String letYdoStuff(Y y, Boolean flag) {
        return y.doStuff(this, flag);
    }
}

class Y {
    public Y(){
    }

    public String doStuff (X x, Boolean flag) {
       String s = "";

       return x.letYdoStuff(this, flag);
   }

   public String doStuff(A a, Boolean flag) {
       System.out.println("in A");
       return "";
   }

   public String doStuff(B b, Boolean flag) {
       System.out.println("in B");
       return "";
   }

   public String doStuff(C c, Boolean flag) {
       System.out.println("in C");
       return "";
   }
}
efan
  • 968
  • 6
  • 11
  • This will not work. There is nothing here that causes any polymorphism to occur. The result is that letYdoStuff and the first Y.doStuff will call each other recursively until the stack overflows. – ajb Sep 19 '13 at 15:18
  • +1 This seems like the right solution... -1 But won't work as intended the way it is currently given. – Dirk Sep 19 '13 at 15:24
  • @ajb you are right, the way it was it would not work. Edited to provide complete example. – efan Sep 19 '13 at 15:35
0

This can be a difficult problem. I think Cruncher's solution, add doStuff to X and override it in A, B, C, is the simplest and best solution when it's appropriate. However, it isn't always appropriate, because of the Single responsibility principle. (I think that's the correct term. My apologies if I get some terminology wrong, I'm not entirely up-to-date on all of the terms.)

The idea is that you shouldn't necessarily doStuff to X if it has nothing to do with the purpose of X. If X and Y are part of the same "team", i.e. they've been both set up to serve the purpose of one particular application, then it's probably OK.

But suppose you have an abstract Shape class that has subclasses Circle, Square, Undecagon, RandomBlob, etc. There will be some methods that belong in the Shape class that would be useful to any application that uses the Shape class. But now say you are writing a game that uses some of those shapes, and you want a polymorphic operation that determines what happens when the shape gets eaten by a flying monkey. You wouldn't want to add an abstract computeEatenByFlyingMonkey method to your Shape class, even if the class were your own creation and not in someone else's library, because that would be just too specific for a class that could be generally used for other purposes than this one game.

I can think of a couple ways to approach this.

If it's not appropriate (or not possible) to add doStuff to X, but if A, B, and C are more closely connected to your application so that adding doStuff to them is appropriate, you can add another class:

public abstract class XWithStuff extends X {
    // repeat any constructors in X, making them all be just
    // calls to super(...)
    public abstract void doStuff (Boolean flag);
}

public class A extends XWithStuff {
    @Override
    public void doStuff (Boolean flag) { ... }
}

and so on for every other class. (XWithStuff is just an example name; in real life, a name that contains both "X" and some reference to the application or purpose is probably better.) (P.S. I don't know why you're using Boolean instead of boolean but I'm leaving it that way in case there's a good reason.)

If it's also not appropriate or not possible to add doStuff to A, B, and C, here's a possible solution:

public interface StuffInterface {
    public void doStuff (Boolean flag);
}

public class AWithStuff extends A implements StuffInterface {
    @Override
    public void doStuff (Boolean flag) { ... }
}

and then in your program create objects of class AWithStuff instead of A, etc. To call doStuff on an X:

void doStuff (X x, Boolean flag) {
    if (x instanceof StuffInterface)  {
        ((StuffInterface) x).doStuff (flag);
    } else {
        throw new IllegalArgumentException ();
    }
}

If that's not an option and you have to deal directly with A, B, etc., and you can't add doStuff to those classes, then any solution will be a bit hacky. If you don't want to use if-then-else, you could look into the visitor pattern, or you could conceivably create a HashMap<Class<?>,Interf> that would map A.class, B.class, etc., to some interface object that calls the correct doStuff. But I haven't worked out the details. (Actually, the "visitor pattern" probably wouldn't be appropriate unless you have some sort of complex structure composed of objects of type X.)

ajb
  • 31,309
  • 3
  • 58
  • 84
0

Separate a DoStuffOperation, create the relative factory and use them.

  public interface DoStuffOperation<T> {
    String doStuff(T x);
  }
  public class ADoStuffImpl implements DoStuffOperation<A> {
    public String doStuff(A x) {
      return "doStuff<A>";
    }
  }
  public class ADoStuffWithFlagImpl implements DoStuffOperation<A> {
    public String doStuff(A x) {
      return "doStuffWithFlag<A>";
    }
  }
  public class DoStuffImplFactory {
    public final static <T extends X> DoStuffOperation<X> getDoStuff(Class<T> xClass,boolean flag)  {
      DoStuffOperation<X> impl = null;

      if(xClass.equals(A.class))
      {
        if(flag)
          impl = (DoStuffOperation)new ADoStuffWithFlagImpl();
        else
          impl = (DoStuffOperation)new ADoStuffImpl();
        }
      }
      return impl;
    }
  }

  public class Y implements InterfaceY {
    public String doStuff (X x, Boolean flag) {
      return DoStuffImplFactory.getDoStuff(x.getClass(),flag).doStuff(x);
  }
}

In this way you don't have to refactor call to Y.doStuff() or X and derived classes. You can't remove at all some sort of instanceof to decide which implementation of doStuff() use unless X classes implements a DoStuffCreator interface like:

interface DoStuffCreator {
  DoStuffOperation getDoStuffOperation(boolean flag);
}

X and A are your classes. You can also construct using reflection or other automatic way (external property file and so on).

Luca Basso Ricci
  • 17,829
  • 2
  • 47
  • 69