Refactoring a huge if (... instanceof ...)

Question

I am currently trying to reorganize a part of the project that looks like this:

Many classes

B extends A; C extends A; D extends C; E extends B; F extends A; ... 

And somewhere in the code:

 if (x instanceof B){ B n = (B) x; ... }else if (x instanceof C){ C n = (C) x; ... }else if (x instanceof D){ D n = (D) x; ... }else if (x instanceof E){ E n = (E) x; ... }else if (x instanceof G){ G n = (G) x; ... }... 

Above if-construct, it currently sits in a function with CC 19. Now my question is: can I separate this if-construct in mutliple functions and let Java OO do the magic? Or are there any catches I should look for?

My idea:

 private void oopMagic(C obj){ ... Do the stuff from the if(x instanceof C) here} private void oopMagic(D obj){ ... Do the stuff from the if(x instanceof D) here} private void oopMagic(E obj){ ... Do the stuff from the if(x instanceof E) here} .... 

and instead of a huge if:

 oopMagic(x); 

Edit: I cannot change any of the classes (A, B, C, ...). Inside if statements, some getters are used to read (never write) data from each object.

Answer 1

It will not fly. instanceof defines the type of the runtime variable. Polymorphism (signature selection method) depends on the type of compilation variable. That is, you always get oopMagic(A obj) .

As Roger suggested, take a look at the template, aka double dispatch

Answer 2

It's hard to say that it will work better since you did not specify what the code does, but another option is to add the oopMagic() method to A and override it as needed.

Answer 3

If you cannot change the classes, you can still wrap them and enter factory to create the correct wrapper for your type:

---

 public interface Wrapper { public void magicMethod(); // you know what I mean } 

---

 public AWrapper implements Wrapper { private A a; public AWrapper(A a){this.a=a;}; @Override public void magicMethod() { // do what has to be done with a } } 

---

 public Factory { public static createWrapper(Object wrappable) { if (wrappable instanceof A) return new AWrapper((A) wrappable); // ... } } 

---

 // ... Object x = getXFromSomewhere(); Wrapper wrapper = Factory.getWrapper(x); wrapper.magicMethod(); // ... 

Of course, this will not eliminate the sequence of instanceof operators, but it will transfer it to the factory and, at least, to the best place. factory methods almost always contain some conditional checks necessary to create the correct object.

Answer 4

I'm afraid (as one commenter also suggests), you are simply moving the problem somewhere else, for example. if-else will execute elsewhere. There is no problem with your approach, but to make it better, you need to do more work.

I suggest creating a map that will process x according to its type. The processor itself is a common interface, for example:

 interface Processor<T extends A> { void oopMagic(T obj); } 

Then create a processor implementation for each class, for example:

 class ProcessorB<B> { void oopMagic(B obj) { ... } } 

Put the entire implementation class on the map, and then do:

 map.get(x.getClass()).oopMagic(x); 

Answer 5

Depending on what is in these ... s, you could get away from creating a virtual (possibly abstract) doMagic method in the base class, and then simply:

 x.doMagic(); 

If the parts ... quite different, then (in addition to other design problems) you can look at the implementation of double sending using the visitor template.