Avoiding use of instance of:

Question

I'm writing a simple game where we have a collection of objects where a player moves around on a grid, collecting coin and avoid monsters.

My class structure looks as follows.

Game Controller - Responsible for spawning coins and tracking game state

Grid - A class which stores the objects which are currently on the grid, and a few grid related methods.

GridObject - An abstract class representing an object on the grid. e.g. player, monster or coin.

Player, Monster, Coin - All extend GridObject.

What is the most OOP way to program collision handling? The desired result is: Player hits Monster - end game, Player hits Coin - increase score, Monster hits Coin - nothing.

At present, when a GridObject moves, it notifies the board object that it wants to move; if this will cause a collision, I call a handler on the GameController (through a listener pattern), to handle the collision. In that handler, which takes two GridObjects as parameters, I'm using a lot of "instanceof" commands to distinguish the above cases.

What is the best way to avoid using all this "instanceof", which I have read usually means bad design?

score 0 · Answer 1 · answered May 29 '15 at 18:01

0

Instead of using instanceof, you could use an enum in GridObject to determine the type. This way, you're just comparing an enum to check which objects are colliding. For example, (warning, syntax may not be exactly correct)

public enum ObjectType {
    Player, Monster, Coin
}

private final ObjectType type;

public GridObject(ObjectType type) {
    this.type = type;
}

public ObjectType getType() {return type;}

And then in your subclasses, you'll have to do the following:

public Player(your parameters) {
    super(ObjectType.Player);
}

And then to check the types, you can just do:

switch(gridObject.getType()) {
    case ObjectType.Player:
       // do something 
       break;
    case ObjectType.Monster:
        // do something else
        break;
    case ObjectType.Coin:
        // do something else
        break;
}

answered May 29 '15 at 18:01

Joseph Roque

5,066
3
16
22

Hi Joseph, thanks for the reply! That's certainly a more elegant solution to what I'm doing at the moment, but logically it's exactly the same as using instanceof, so is it maybe hiding bad design rather than solving the problem? – WMycroft May 29 '15 at 18:07
1

@WMycroft Perhaps you're right, I did a bit of searching and it seems the preferred method, if you can, is to create an abstract method in your superclass and have each subclass implement it differently. See [this question](http://stackoverflow.com/questions/5579309/switch-instanceof) for an example. However, if that's not possible (which it doesn't seem like it will be for your situation), then as you can see from that same question, a enum is a reasonable alternative. – Joseph Roque May 29 '15 at 18:29
I guess the main issue we are having is that collision is really a property of **two** objects, and so there is not going to be a suitable place we can put an abstract method in the heirarchy... – WMycroft May 29 '15 at 18:40
`instanceof` is generally bad because code is bound to an implementation detail (a class name). A polymorphic method here (`getType()`) is less fragile. I would use a different name for the enum, say `CollisionClass`, and change the method to `getCollisionClass()` since this information is related to collisions. I prefer this answer to the Amir's because the logic for collisions is kept in a centralized place. In the other answer, each object decides what happens after it interacts with another. That couples the GridObjects and I makes it harder to see how collisions work globally. – Fuhrmanator May 30 '15 at 01:43

score 0 · Accepted Answer · answered May 29 '15 at 18:06

0

What about introducing some interfaces :

interface Interactable { 

   InteractionResult interact(WorldEntity entity);

}

interface WorldEntity {

    boolean isMonster();

    boolean isPlayer();

    boolean isPowerUp();

}

Now we can say that when a Player interacts with other things we can do :

   public InteractionResult interact(WorldEntity entity) {
       if(entity.isMonster()) {
           return new DeathInteraction(this);
       }
       ...
   }

You don't eliminate all of the conditionals, but at least you start to organize your abstractions and decouple your classes.

answered May 29 '15 at 18:06

Amir Afghani

37,814
16
84
124

This is a nice solution, something feels a bit uneasy to me about implementing this with my current code, but I can't put my finger on it right now... I'll comment again if I get any problems – WMycroft May 29 '15 at 18:21
Doesn't this mean every class that implements that interface will have to override all of those boolean methods, returning false for all of them except one? I feel like this is much messier than even just leaving the instanceof calls. – Joseph Roque May 29 '15 at 18:31
You can have abstract classes with default implementations – Amir Afghani May 29 '15 at 18:32
There is some good stuff in here, which seems to me to be logically the same as your suggestion: http://gamedev.stackexchange.com/questions/77701/how-to-handle-collisions-without-ugly-conditionals-and-type-checking – WMycroft May 29 '15 at 19:22
This method requires the knowledge of what happens to be spread out within the classes. I'm not sure I agree with a Player class deciding it dies if it hits a Monster (or vice versa). This solution means that the game logic is spread out. Perhaps that is OK. – Fuhrmanator May 30 '15 at 01:16

Avoiding use of instance of:

2 Answers2