How to avoid instanceof operator in this case

Question

I'm writing simple game with cells which can be in two states Free and Taken by player.

interface Cell {
    int posX();
    int posY();
}

abstract class BaseCell implements Cell {

    private int x;
    private int y;

    public int posX() {
        return x;
    }

    public int posY() {
        return y;
    }

    ...
}

class FreeCell extends BaseCell {
}

class TakenCell extends BaseCell {
    private Player owningPlayer

    public Player owner() {
        return owningPlayer;
    }

}

In each turn I need to inspect all cells to calculate next cell state with method as below

// method in class Cell
public Cell nextState(...) {...}

and collect (in Set) all cells that are not yet taken. The method above returns Cell because cell may change from Free to Taken or the opposite. I'm doing something like below to collect them:

for (Cell cell : cells) {
    Cell next = cell.futureState(...);
    if(next instanceof FreeCell) {
        freeCells.add(currentCell);
    }
    ...
}

It's ugly. How to do that to avoid such instanceof hacks? I'm not talking about another hack, but would like to find out proper OOP solution.

Answer 1

It sounds like you are flirting with the "State" pattern but you are not quite there. Using the state pattern you would have your Cell object and a hierarchy of "Cell State" classes.

The Cell object would use composition rather than inheritance. In other words, a Cell would have a current state property. When you have a Cell where the currentState property is a FreeState object then it's a free cell. When you have a Cell where the currentState property is a TakenState object, then it's a free state.

How to do that to avoid such instanceof hacks?

Whenever you have a situation where you would need to do an instanceof, you add a method to your Cell class and just invoke it. The Cell delegates to the current state. The code in Cell which delegates to the current state does not actually know what the state is. It just trusts that the state will do the right thing. In your FreeState and TakenState you provide implementations of each method that do the right thing based on their state.

Answer 2

I think the design problem here is that you have two different classes for what can be essentially two different states of the same cell.

What do you do now when a previously free cell becomes occupied? Create a new object with same coordinates and discard the old one? But it's still the same cell conceptually! (Or can there be a free cell and a taken cell with same x and y at the same time?)

From the OOP perspective, you should have one cell class with an attribute "taken", or as another anwer suggests, "owner information". If you feel that this should not be part of the cell class for whatever reason, what about keeping the owner information separate in a Map<Cell,Owner>?