code

Java에서 instanceof 피하기

codestyles 2020. 8. 27. 07:48
반응형

Java에서 instanceof 피하기


일련의 "instanceof"작업을 갖는 것은 "코드 냄새"로 간주됩니다. 표준 대답은 "다형성 사용"입니다. 이 경우 어떻게해야합니까?

기본 클래스에는 여러 하위 클래스가 있습니다. 그들 중 누구도 내 통제하에 있지 않습니다. 유사한 상황은 Java 클래스 Integer, Double, BigDecimal 등입니다.

if (obj instanceof Integer) {NumberStuff.handle((Integer)obj);}
else if (obj instanceof BigDecimal) {BigDecimalStuff.handle((BigDecimal)obj);}
else if (obj instanceof Double) {DoubleStuff.handle((Double)obj);}

나는 NumberStuff 등을 제어 할 수 있습니다.

몇 줄로 할 수있는 코드 줄을 많이 사용하고 싶지 않습니다. (때로는 Integer.class를 IntegerStuff의 인스턴스로, BigDecimal.class를 BigDecimalStuff의 인스턴스로 매핑하는 HashMap을 만듭니다.하지만 오늘은 더 간단한 것을 원합니다.)

다음과 같이 간단한 것을 원합니다.

public static handle(Integer num) { ... }
public static handle(BigDecimal num) { ... }

그러나 Java는 그렇게 작동하지 않습니다.

서식을 지정할 때 정적 메서드를 사용하고 싶습니다. 내가 서식을 지정하는 것은 복합적이며 Thing1은 Thing2 배열을 포함 할 수 있고 Thing2는 Thing1 배열을 포함 할 수 있습니다. 다음과 같은 포맷터를 구현할 때 문제가 발생했습니다.

class Thing1Formatter {
  private static Thing2Formatter thing2Formatter = new Thing2Formatter();
  public format(Thing thing) {
      thing2Formatter.format(thing.innerThing2);
  }
}
class Thing2Formatter {
  private static Thing1Formatter thing1Formatter = new Thing1Formatter();
  public format(Thing2 thing) {
      thing1Formatter.format(thing.innerThing1);
  }
}

예, 저는 HashMap을 알고 있으며 더 많은 코드로도 해결할 수 있습니다. 그러나 "instanceof"는 비교해 보면 매우 읽기 쉽고 유지 관리가 가능해 보입니다. 간단하지만 냄새가 나지 않는 것이 있습니까?

2010 년 5 월 10 일에 추가 된 메모 :

새 하위 클래스가 향후 추가 될 것이며 기존 코드가이를 정상적으로 처리해야 할 것입니다. 클래스를 찾을 수 없기 때문에 클래스의 HashMap은이 경우 작동하지 않습니다. 가장 구체적인 것으로 시작하고 가장 일반적인 것으로 끝나는 if 문 체인은 결국 가장 좋습니다.

if (obj instanceof SubClass1) {
    // Handle all the methods and properties of SubClass1
} else if (obj instanceof SubClass2) {
    // Handle all the methods and properties of SubClass2
} else if (obj instanceof Interface3) {
    // Unknown class but it implements Interface3
    // so handle those methods and properties
} else if (obj instanceof Interface4) {
    // likewise.  May want to also handle case of
    // object that implements both interfaces.
} else {
    // New (unknown) subclass; do what I can with the base class
}

You might be interested in this entry from Steve Yegge's Amazon blog: "when polymorphism fails". Essentially he's addressing cases like this, when polymorphism causes more trouble than it solves.

The issue is that to use polymorphism you have to make the logic of "handle" part of each 'switching' class - i.e. Integer etc. in this case. Clearly this is not practical. Sometimes it isn't even logically the right place to put the code. He recommends the 'instanceof' approach as being the lesser of several evils.

As with all cases where you are forced to write smelly code, keep it buttoned up in one method (or at most one class) so that the smell doesn't leak out.


As highlighted in the comments, the visitor pattern would be a good choice. But without direct control over the target/acceptor/visitee you can't implement that pattern. Here's one way the visitor pattern could possibly still be used here even though you have no direct control over the subclasses by using wrappers (taking Integer as an example):

public class IntegerWrapper {
    private Integer integer;
    public IntegerWrapper(Integer anInteger){
        integer = anInteger;
    }
    //Access the integer directly such as
    public Integer getInteger() { return integer; }
    //or method passthrough...
    public int intValue() { return integer.intValue(); }
    //then implement your visitor:
    public void accept(NumericVisitor visitor) {
        visitor.visit(this);
    }
}

Of course, wrapping a final class might be considered a smell of its own but maybe it's a good fit with your subclasses. Personally, I don't think instanceof is that bad a smell here, especially if it is confined to one method and I would happily use it (probably over my own suggestion above). As you say, its quite readable, typesafe and maintainable. As always, keep it simple.


Instead of a huge if, you can put the instances you handle in a map (key: class, value: handler).

If the lookup by key returns null, call a special handler method which tries to find a matching handler (for example by calling isInstance() on every key in the map).

When a handler is found, register it under the new key.

This makes the general case fast and simple and allows you to handle inheritance.


You can use reflection:

public final class Handler {
  public static void handle(Object o) {
    try {
      Method handler = Handler.class.getMethod("handle", o.getClass());
      handler.invoke(null, o);
    } catch (Exception e) {
      throw new RuntimeException(e);
    }
  }
  public static void handle(Integer num) { /* ... */ }
  public static void handle(BigDecimal num) { /* ... */ }
  // to handle new types, just add more handle methods...
}

You can expand on the idea to generically handle subclasses and classes that implement certain interfaces.


You could consider the Chain of Responsibility pattern. For your first example, something like:

public abstract class StuffHandler {
   private StuffHandler next;

   public final boolean handle(Object o) {
      boolean handled = doHandle(o);
      if (handled) { return true; }
      else if (next == null) { return false; }
      else { return next.handle(o); }
   }

   public void setNext(StuffHandler next) { this.next = next; }

   protected abstract boolean doHandle(Object o);
}

public class IntegerHandler extends StuffHandler {
   @Override
   protected boolean doHandle(Object o) {
      if (!o instanceof Integer) {
         return false;
      }
      NumberHandler.handle((Integer) o);
      return true;
   }
}

and then similarly for your other handlers. Then it's a case of stringing together the StuffHandlers in order (most specific to least specific, with a final 'fallback' handler), and your despatcher code is just firstHandler.handle(o);.

(An alternative is to, rather than using a chain, just have a List<StuffHandler> in your dispatcher class, and have it loop through the list until handle() returns true).


I think that the best solution is HashMap with Class as key and Handler as value. Note that HashMap based solution runs in constant algorithmic complexity θ(1), while the smelling chain of if-instanceof-else runs in linear algorithmic complexity O(N), where N is the number of links in the if-instanceof-else chain (i.e. the number of different classes to be handled). So the performance of HashMap based solution is asymptotically higher N times than the performance of if-instanceof-else chain solution. Consider that you need to handle different descendants of Message class differently: Message1, Message2, etc. . Below is the code snippet for HashMap based handling.

public class YourClass {
    private class Handler {
        public void go(Message message) {
            // the default implementation just notifies that it doesn't handle the message
            System.out.println(
                "Possibly due to a typo, empty handler is set to handle message of type %s : %s",
                message.getClass().toString(), message.toString());
        }
    }
    private Map<Class<? extends Message>, Handler> messageHandling = 
        new HashMap<Class<? extends Message>, Handler>();

    // Constructor of your class is a place to initialize the message handling mechanism    
    public YourClass() {
        messageHandling.put(Message1.class, new Handler() { public void go(Message message) {
            //TODO: IMPLEMENT HERE SOMETHING APPROPRIATE FOR Message1
        } });
        messageHandling.put(Message2.class, new Handler() { public void go(Message message) {
            //TODO: IMPLEMENT HERE SOMETHING APPROPRIATE FOR Message2
        } });
        // etc. for Message3, etc.
    }

    // The method in which you receive a variable of base class Message, but you need to
    //   handle it in accordance to of what derived type that instance is
    public handleMessage(Message message) {
        Handler handler = messageHandling.get(message.getClass());
        if (handler == null) {
            System.out.println(
                "Don't know how to handle message of type %s : %s",
                message.getClass().toString(), message.toString());
        } else {
            handler.go(message);
        }
    }
}

More info on usage of variables of type Class in Java: http://docs.oracle.com/javase/tutorial/reflect/class/classNew.html


Just go with the instanceof. All the workarounds seem more complicated. Here is a blog post that talks about it: http://www.velocityreviews.com/forums/t302491-instanceof-not-always-bad-the-instanceof-myth.html


I have solved this problem using reflection (around 15 years back in pre Generics era).

GenericClass object = (GenericClass) Class.forName(specificClassName).newInstance();

I have defined one Generic Class ( abstract Base class). I have defined many concrete implementations of base class. Each concrete class will be loaded with className as parameter. This class name is defined as part of configuration.

Base class defines common state across all concrete classes and concrete classes will modify the state by overriding abstract rules defined in base class.

At that time, I don't know the name of this mechanism, which has been known as reflection.

Few more alternatives are listed in this article : Map and enum apart from reflection.

참고URL : https://stackoverflow.com/questions/2790144/avoiding-instanceof-in-java

반응형