When is it acceptable to use instanceof? [closed]

问题:

I'm designing a game. In the game, various game objects extend different interfaces (and one abstract class) depending on what they need to be doing, and are passed to handlers which take care of items with a specific interface at defined intervals (they actually spread all their work out in a neat sort of way to make sure input/video/etc is always processed).

Anyway, some of these objects extend the abstract class Collider and are passed to a CollisionHandler. The Collider class and handler take care of everything technical involved in collision, and just ask that an object implement a collidesWith(Collider c) function, and modify itself based on what it has collided with.

Objects of many different classes will be colliding with one another, and will act very differently depending on what type of object they have collided with and its specific attributes.

The perfect solution seems to be to use instanceof like so:

class SomeNPC extends Collider{
    collidesWith(Collider c){
        if(c instanceof enemy){
            Fight it or run away depending on your attributes and theirs.
        }
        else if(c instanceof food){
            Eat it, but only if it's yellow.
        }
        else if(c instanceof BeamOfLight){
            Try to move towards its source.
        }
    }
}

This actually seems like a legitimate place for instanceof. I just get this bad feeling. Like how if a goto made sense in some particular situation. Does the design feel fundamentally off to anyone? If so, what would you recommend doing to achieve the same behavior.

回答1:

Traditional answer is, use a Visitor pattern. You add a new interface,

interface Visitor {
     void visit(Enemy e);
     void visit(Food f);
     void visit(BeanOfLight bol);
}

and a method,

public void visit(Visitor v) {
    visitor.visit(this);
}

Every object in your game implements a visit method, and every action you need implements a Visitor interface. So as soon as action visits an object, it is forced to do an action associated with that object.

You can of course be more detailed and not rely on the method dispatching mechanism.

Update: returning to the question header, it is always acceptable to use instanceof. It's your code, it's your language to use. Problem is, if there are many places in your code where you use instanceof, you'll inevitably miss one sooner or later, so that your code will silently fail without compiler there to help you. Visitor will make your life more painful during coding, as it will force you to implement the interface each time you change it, everywhere. But on a plus side, you won't miss a case this way.

Update 2: Please read the discussion below. Visitor will of course bind you and you'll feel constrained by it as soon as you have more that a dozen types. Moreover, if you need to dispatch events, e.g. collisions, basing on types of two or more objects, no visitor will help you (no instanceof, either): you will need to implement your own table of collision consequences which would map your type combinations to an object (I'd say Strategy, but am afraid discussion will grow tenfold) which would know how to deal with this particular collision.

An obligatory Stroustrup quote: "There is no substitute for: Intelligence; Experience; Taste; Hard work."

回答2:

The visitor class is often recommended. With Visitor you implement a visit method:

interface Visitor {
 void visit(Enemy e);
 void visit(Food f);
 void visit(BeanOfLight bol);
}

But this is in fact equivalent to:

class SomeNPC extends Collider {
  public void collidesWith( Enemy enemy )
  public void collidesWith( Food food )
  public void collidesWith( Bullet bullet )
}

Both of these have disadvantages.

1. You have to implement all of them, even if the response of you object is the same in each case
2. If you add a new kind of object to collide with you have to write a method to implement colliding with it for every single object.
3. If one object in your system reacts differently to 27 kinds of colliders, but everything else reacts the same way, you still have to write 27 visitor methods for every class.

Sometimes the easiest way is to do:

collidesWith(Object o) {
  if (o instanceof Balloon) {
    // bounce
  } else {
    //splat
  }

It has the advantage that it keeps the knowledge of how an object reacts to things it hits with that object. it also means that if Balloon has subclasses RedBalloon, BlueBalloon etc we don't have to take that into account, as we would do with the visitor pattern.

The traditional argument for not using instanceof is that it isn't OO, and you should be using polymorphism. However you might be interested in this article: When Polymorphism Fails, by Steve Yegge which explains why instanceof is sometimes the right answer.

回答3:

It's strange no one have posted a "not broken" visitor pattern implementation yet. And by not broken I mean not relying on the side effects of the visitor. To do that, we need our visitors to return some result (let's call it R):

interface ColliderVisitor<R> {
     R visit(Enemy e);
     R visit(Food f);
     R visit(BeanOfLight bol);
     R visit(SomeNpc npc);
}

Next, we modify accept to accept the new visitors:

interface Collider {
    <R> R accept(ColliderVisitor<R> visitor);
}

the concrete implementations of collider will have to call the proper visit method, like this (I'm assuming Food implements Collider, but this is not necessary):

class Food implements Collider {
    @Override
    <R> R accept(ColliderVisitor<R> visitor) {
        return visitor.visit(this);
    }
}

Now to implement the collisions we can do something like this:

class SomeNpcCollisionVisitor implements ColliderVisitor<Action> {
    SomeNpcCollisionVisitor(SomeNpc me) { this.me = me; }
    SomeNpc me;
    @Override
    Action visit(Enemy they) { 
        return fightItOrRunAway(me.attributes(), they.attributes());
    }
    @Override
    Action visit(Food f) {
        return f.colour()==YELLOW ? eat(f) : doNothing;
    }
    @Override
    Action visit(BeamOfLight l) {
        return moveTowards(l.source());
    }
    @Override
    Action visit(SomeNpc otherNpc) {
       // What to do here? You did not say! The compiler will catch this thankfully.
    }
}

class CollisionVisitor implements 
        ColliderVisitor<ColliderVisitor<Action>> { // currying anyone?

    @Override
    Action visit(Enemy they) { 
        return new EnemyCollisionVisitor(they); // what to do here?
    }
    @Override
    Action visit(Food f) {
        return new FoodCollisionVisitor(f); // what to do here?
    }
    @Override
    Action visit(BeamOfLight l) {
        return new BeamOfLightCollisionVisitor(l); // what to do here?
    }
    @Override
    Action visit(SomeNpc otherNpc) {
       return new SomeNpcCollisionVisitor(otherNpc);
    }
}

Action collide(Collider a, Collider b) {
    return b.accept(a.accept(new CollisionVisitor()));
}

You can see that the compiler can help you find all the places where you forgot to specify the behaviour. This is not a liability as some people claim, but advantage because you can always disable it by using a default implementation:

class ColliderVisitorWithDefault<R> implements ColliderVisitor {
    final R def;
    ColliderVisitorWithDefault(R def) { this.def = def; }
    R visit(Enemy e) { return def; }
    R visit(Food f) { return def; }
    R visit(BeanOfLight bol) { return def; }
    R visit(SomeNpc npc) { return def; }
}

You will also want some way to reuse the code for collisions of (Food, SomeNpc) and (SomeNpc, Food), but this is outside the scope of this question.

If you think this is too verbose -- that's because it is. In languages featuring pattern matching this can be done in several lines (Haskell example):

data Collider = 
    Enemy <fields of enemy>
  | Food <fields of food>
  | BeanOfLight <fields>
  | SomeNpc <fields>

collide (SomeNpc npc) (Food f) = if colour f == YELLOW then eat npc f else doNothing
collide (SomeNpc npc) (Enemy e) = fightOrRunAway npc (npcAttributes npc) (enemyAttributes e)
collide (SomeNpc npc) (BeamOfLight bol) = moveTowards (bolSource bol)
collide _ _ = undefined -- here you can put some default behaviour

回答4:

You can use a visitor pattern here where subclasses of Collider would implement a separate method for each type of collision it can encounter. So you method might turn into:

class SomeNPC extends Collider {

    public void collidesWith( Enemy enemy ) {}

    public void collidesWith( Food food ) {}

    public void collidesWith( Bullet bullet ) {}

    public void doCollision( Collider c ) {
        if( c.overlaps( this ) ) {
            c.collidesWith( this );
        }
    }
}

You get the idea. The strange thing in your model is that a Collider base class has to know about all potential subclasses in order to define a method for that type. Part of this has to do with the problem of the visitor pattern, but it's also because Collider is combined into Visitor. I'd suggest looking for a separation of the visitor and collider so you can define how you want to behave when collisions happen. What that means to your colliders is they can change how they behave to a collision based on internal state. Say they are invulnerable vs. normal mode, hidden, or dead. Looking at the client code it might be:

collider1.getCollisionVisitor().doCollision( collider2 );
collider2.getCollisionVisitor().doCollision( collider1 );

回答5:

In my opinion, what you have sketched above is a legitimate use of instanceof, and may be more readable than using a Visitor system, if each class only interacts with a few other classes as seen above.

The problem is that it has the potential to turn into pages of else-ifs for each of twenty types of enemies. But with instanceof, you can avoid that with some standard use of polymorphism (check for one Enemy class and treat all enemies alike, even if they're Orcs or Daleks or whatnot).

The Visitor pattern makes it much harder to do that. The most workable solution would be to have one top-level class that all your game objects derive from, and define collideWith() methods in that class for all its subclasses - but then have the default implementation of each just call the collideWith() for the supertype:

class GameObject {
   void collideWith(Orc orc) {
      collideWith((Enemy)orc);
   }

   void collideWith(Enemy enemy) {
      collideWith((GameObject)enemy);
   }

   ...

   void collideWith(GameObject object) { }
}

class SomeNPC extends GameObject {
   void collideWith(Orc orc) {
      // Handle special case of colliding with an orc
   }

   // No need to implement all the other handlers,
   // since the default behavior works fine.
}

回答6:

This is a use of instanceof that will make most people cringe. Here is a useful link that may provide you with some insight: http://www.javapractices.com/topic/TopicAction.do?Id=31

Instead, a Collider should have some sort of collide() method that each subclass will override.

回答7:

This might be something to consider. Using instanceof can be useful but another way to consider the visitor idea is to build a Factory class that examines a mutual field to determine what type the visitor is.

You still have to build new classes for each implementation, but they are routed in an abstract class that defines a method

 public abstract class Qualifier{
 public abstract String type(); //...
 }

/**
 * QualifierFactory method.
 */
public static Qualifier getQualifier(Item sp, Config cf) {
    String type = cf.type();
    if (XQualifier.type().equals(type))
        return new XQualifier(sp, cf);
    else if (NQualifier.type().equals(type))
        return new NQualifier(sp, cf);
    else if (Tools.isNone(type) || NoneQualifier.type().equals(type))
        return new NoneQualifier(sp);
    else if (CSQualifier.type().equals(type))
        return new CSQualifier(sp, cf);//...
 }

The returned object could be the action .

回答8:

There are a few ways to handle it this:

• enums for the collider types, slightly ugly but fail-proof
• class dispatch with handlers: i.e. something like Map<Class, CollisionHandler>, you pick the CollisionHandler off the passed collider class (or enum type) and call processCollision(source). Each Colllider class has its own map of handlers.
• Based on the above you can also create a combination of Colliders types and write handlers for, that's something like Map<Pair<ColliderType>, CollisionHandler> for each new collision type you'd need to create a new handler. The plus side is that such declarations can be external (dependency injection), so new NPCs/Object can be added along with the Collision handlers.

Anyways first you make sure it works the way you feel the most comfortable with and you can refactor it later.