avoid instanceof in Java

Question

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!

Answer 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);
   }
}

Answer 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.