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

I'm currently trying to refactor a part of a 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 inst开发者_JAVA百科anceof 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 currently sits in a function with a CC of 19. Now my question is: Can I split this if-construct up in mutliple functions and let Java's OO do the magic? Or are there any catches I have to look out 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 the huge if:

oopMagic(x);

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

That won't fly. instanceof detects the runtime type of a variable. Polymorphism (select method by signature) depends on the compile-time type of a variable. That is, you'll always get oopMagic(A obj).

As Roger suggested, take a look at the visitor pattern, a.k.a double-dispatch.

It's hard to tell what would work best since you haven't given any indication what the code is doing, but another option is to add an oopMagic() method to A and override as necessary.

If you can't change the the classes, you may still be able to wrap them and introduce a factory to create a 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();
// ...

Sure, this will not eliminate the sequence of instanceof statements but it will move it to a factory and that's at least a better place. Factory methods almost always contain some conditional checks which are needed to create the correct object.

I'm afraid (as also suggested by one commenter) you just move the problem somewhere else, e.g. the if-else will be done in other place. There is no problem with your approach but to make it better you have to do some more work.

What I suggest is to create a Map that will process x according to its type. Processor itself is a generic interface, something like:

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

Then create implementation of the processor for each class, something like:

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

Put all implementation class in the map and then do:

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

Depending on what's in those ...s, you could get away with making a virtual (maybe abstract) method doMagic in the base class and then just:

x.doMagic();

If the ... parts are sufficiently different, then (apart from having other problems with the design), you could look at implementing double-dispatch using the visitor pattern.