avoid instanceof in Java

问题:

I have been told at some stage at university (and have subsequently read in upteen places) that using instanceof should only be used as a 'last resort'. With this in mind, is anyone able to tell be if the following code I have is a last resort. I have had a look around on stack overflow but cannot quite find a similar scenario - perhaps I have missed it?

private void allocateUITweenManager() {
   for(GameObject go:mGameObjects){
      if (go instanceof GameGroup) ((GameGroup) go).setUITweenManager(mUITweenManager);
   }
}

where

• mGameObjects is an array, only some of which are GameGroup type
• GameGroup is a subclass of abstract class GameObject.
• GameGroup uses interface UITweenable which has method setUITweenManager()
• GameObject does not use interface UITweenable

I suppose I could equally (and probably should) replace GameGroup in my code above with UITweenable - I would be asking the same question.

Is there another way of doing this that avoids the instanceof? This code cannot fail, as such (I think, right?), but given the bad press instanceof seems to get, have I committed some cardinal sin of OOP somewhere along the line that has me using instanceof here?

Thanks in advance!

回答1:

I learned about Visitor pattern in Compiler class at university, I think it might apply in your scenario. Consider code below:

public class GameObjectVisitor {

    public boolean visit(GameObject1 obj1) { return true; }
    .
    .
    // one method for each game object
    public boolean visit(GameGroup obj1) { return true; }
}

And then you can put a method in GameObject interface like this:

public interface GameObject {

    .
    .
    public boolean visit(GameObjectVisitor visitor);
}

And then each GameObject implements this method:

public class GameGroup implements GameObject {

    .
    .
    .
    public boolean visit(GameObjectVisitor visitor) {
        visitor.visit(this);
    }
}

This is specially useful when you've complex inheritance hierarchy of GameObject. For your case your method will look like this:

private void allocateUITweenManager() {

    GameObjectVisitor gameGroupVisitor = new GameObjectVisitor() {
        public boolean visit(GameGroup obj1) {
            obj1.setUITweenManager(mUITweenManager);
        }
    };

    for(GameObject go:mGameObjects){
      go.visit(gameGroupVisitor);
   }
}

回答2:

EDIT

There are two primary things you can do here to relieve yourself of this specific instance of instanceof. (pun?)

1. Do as my initial answer suggested and move the method you are targeting up to the class you are iterating. This isn't ideal in this case, because the method doesn't make sense to the parent object, and would be polluting as Ted has put it.

2. Shrink the scope of the objects you are iterating to just the objects that are familiar with the target method. I think this is the more ideal approach, but may not be workable in the current form of your code.

Personally, I avoid instanceof like the plague, because it makes me feel like I completely missed something, but there are times where it is necessary. If your code is laid out this way, and you have no way to shrink the scope of the objects you are iterating, then instanceof will probably work just fine. But this looks like a good opportunity to see how polymorphism can make your code easier to read and maintain in the future.

I am leaving the original answer below to maintain the integrity of the comments.

/EDIT

Personally, I don't think this is a good reason to use instanceof. It seems to me that you could utilize some polymorphism to accomplish your goal.

Have you considered making setUITweenManager(...) a method of GameObject? Does it make sense to do this?

If it does make sense, you could have your default implementation do nothing, and have your GameGroup override the method to do what you want it to do. At this point, your code could just look like this then:

private void allocateUITweenManager() {
   for(GameObject go:mGameObjects){
       go.setUITweenManager(mUITweenManager);
   }
}

This is polymorphism in action, but I am not sure it would be the best approach for your current situation. It would make more sense to iterate the Collection of UITweenable objects instead if possible.

回答3:

The reason why instanceof is discouraged is because in OOP we should not examine object's types from outside. Instead, the idiomatic way is to let object themselves act using overriden methods. In your case, one possible solution could be to define boolean setUITweenManager(...) on GameObject and let it return true if setting the manager was possible for a particular object. However if this pattern occurs in many places, the top-level classes can get quite polluted. Therefore sometimes instanceof is "lesser evil".

The problem with this OPP approach is that each object must "know" all its possible use cases. If you need a new feature that works on your class hierarchy, you have to add it to the classes themselves, you can't have it somewhere separate, like in a different module. This can be solved in a general way using the visitor pattern, as others suggested. The visitor pattern describes the most general way to examine objects, and becomes even more useful when combined with polymorphism.

Note that other languages (in particular functional languages) use a different principle. Instead of letting objects "know" how they perform every possible action, they declare data types that have no methods on their own. Instead, code that uses them examines how they were constructed using pattern matching on algebraic data types. As far as I know, the closest language to Java that has pattern matching is Scala. There is an interesting paper about how Scala implements pattern matching, which compares several possible approaches: Matching Objects With Patterns. Burak Emir, Martin Odersky, and John Williams.

Data in object-oriented programming is organized in a hierarchy of classes. The problem of object-oriented pattern matching is how to explore this hierarchy from the outside. This usually involves classifying objects by their run-time type, accessing their members, or determining some other characteristic of a group of objects. In this paper we compare six different pattern matching techniques: object-oriented decomposition, visitors, type-tests/typecasts, typecase, case classes, and extractors. The techniques are compared on nine criteria related to conciseness, maintainability and performance. The paper introduces case classes and extractors as two new pattern-matching methods and shows that their combination works well for all of the established criteria.

In summary: In OOP you can easily modify data types (like add subclasses), but adding new functions (methods) requires making changes to many classes. With ADT it's easy to add new functions, but modifying data types requires modifying many functions.

回答4:

The problem with instanceof is that you can suffer from future object hierarchy changes. The better approach is to use Strategy Pattern for the cases where you are likely to use instanceof. Making a solution with instanceof you are falling into a problem Strategy is trying to solve: to many ifs. Some guys have founded a community. Anti-IF Campaign could be a joke but untipattern is serious. In a long term projects maintaining 10-20 levels of if-else-if could be a pain. In your case you'd better make a common interface for all objects of your array and implement setUITweenManager for all of them through an interface.

interface TweenManagerAware{
   setUITweenManager(UITweenManager manager);
}

回答5:

It is always a bit "fishy" to me to mix objects of different classes in the same Collection. Would it be possible / make sense to split the single Collection of GameObjects into multiple Collections, one of mere GameObjects, another of UITweenables? (e.g. use a MultiMap keyed by a Class). Then you could go something like:

for (UITweenable uit : myMap.get(UITweenable.class)) {
  uit.setUITweenManager(mUITweenManager);
}

Now, you still need an instanceof when you insert into the map, but it's better encapsulated - hidden from the client code who doesn't need to know those details

p.s. I'm not a fanatic about all the SW "rules", but Google "Liskov Substitution Principle".

回答6:

You could declare setUITweenManager in GameObject with an implementation that does nothing.

You could create an method that returns an iterator for all UITweenable instances in array of GameObject instances.

And there are other approaches that effectively hide the dispatching within some abstraction; e.g. the Visitor or Adapter patterns.

---

... have I committed some cardinal sin of OOP somewhere along the line that has me using instanceof here?

Not really (IMO).

The worst problem with instanceof is when you start using it to test for implementation classes. And the reason that is particularly bad is that it makes it hard to add extra classes, etcetera. Here the instanceof UITweenable stuff doesn't seem to introduce that problem, because UITweenable seems to be more fundamental to the design.

---

When you make these sorts of judgement, it is best to understand the reasons why the (supposedly) bad construct or usage is claimed to be bad. Then you look at you specific use-case and make up whether these reasons apply, and whether the alternatively you are looking at is really better in your use-case.

回答7:

You could use the mGameObjects container for when you need to do something on all game objects and keep a separate container only for GameGroup objects.

This will use some more memory, and when you add/remove objects you have to update both containers, but it shouldn't be a noticeable overhead, and it lets you loop very efficiently through all the objects.

回答8:

The problem with this approach is that it doesn't usually appear at one place only in your code and thus makes it more or less painful to add another implementations of the interface in the future. Whether to avoid it depends on your consideration. Sometimes YAGNI can be applied an this is the most straightforward way.

Alternatives had been suggested by others, for example the Visitor pattern.

回答9:

I have another suggestion of a way to avoid instanceof.

Unless you are using a generic factory, at the moment when you create a GameObject you know what concrete type it is. So what you can do is pass any GameGroups you create an observable object, and allow them to add listeners to it. It would work like this:

public class Game {
    private void makeAGameGroup() {
        mGameObjects.add(new GameGroup(mUITweenManagerInformer));
    }

    private void allocateUITweenManager() {
        mUITweenManagerInformer.fire(mUITweenManager);
    }

    private class OurUITweenManagerInformer extends UITweenManagerInformer {
        private ArrayList<UITweenManagerListener> listeners;

        public void addUITweenManagerListener(UITweenManagerListener l) {
            listeners.add(l);
        }

        public void fire(UITweenManager next) {
            for (UITweenManagerListener l : listeners)
                l.changed(next);
        }
    }
    private OurUITweenManagerInformer mUITweenManagerInformer = new OurUITweenManagerInformer();
}

public interface UITweenManagerInformer {
    public void addUITweenManagerListener(UITweenManagerListener l);
}

public interface UITweenManagerListener {
    public void changed(UITweenManager next);
}

What draws me to this solution is:

• Because a UITweenManagerInformer is a constructor parameter to GameGoup, you cannot forget to pass it one, whereas with an instance method you might forget to call it.

• It makes intuitive sense to me that information that an object needs (like the way a GameGroup needs knowledge of the current UITweenManager) should be passed as a constructor parameter -- I like to think of these as prerequisites for an object existing. If you don't have knowledge of the current UITweenManager, you shouldn't create a GameGroup, and this solution enforces that.

• instanceof is never used.