Should I really avoid instanceof like the plague?

Question

I'm developing a board game which involves putting pieces in cells. The catch is you can put pieces anywhere in the board (including occupied cells) as long as there isn't any opponent's piece in there.

Right now I've thought about two alternatives to do this:

• Without instanceof:

RedPiece.class snippet

public boolean isAlly(RedPiece p) {
    return true;
}

public boolean isAlly(BluePiece p) {
    return false;
}

public boolean isAlly(GreenPiece p) {
    return false;
}

Obviously, if I want to add a new colour, I'd have to add to each class another method.

• With instanceof:

Piece.class snippet

public boolean isAlly(Piece that) { 
    return this instanceof that;
}

This later approach seems better to me, since I don't need to do anything when adding a new colour.

Of course for my game this wouldn't be an issue since all pieces are the same and I can just make an enum instead of new classes. But what if the red team was suddenly given special abilities which blue and green do not have?

Perhaps the solution is to add an attribute with the string stating which team they belong to (red player and blue player) but right now it's designed so red pieces belong only to the red team. The string "RED" is definitely redundant information which you could get from the class.

In conclusion: Is the usage of instanceof justified here? Can anyone think of a better approach?

Answer 1

Your code without instanceof doesn't make much sense: you won't be able to pass a Piece as argument to any of the methods.

Your code with instanceof doesn't compile, because instanceof expects a class name, not an object. I'm not sure if having subclasses for your pieces is a good idea. If you just need them to have another state, and not another behavior, then you should just have a color field in Piece. Anyway, you could simply implement what you want with the following:

public abstract class Piece {
    public abstract String getColor();

    public final boolean isAlly(Piece otherPiece) {
        return this.getColor().equals(otherPiece.getColor());
    }
}

public class RedPiece extends Piece {
    @Override
    public String getColor() {
        return "red";
    }
}

If you don't have any getColor() method, you can use this.getClass().equals(otherPiece.getClass()), but that prevents you from having a BigRedPiece subclass, for example.

Answer 2

I think you should add a flag or ally variable to your super class Piece, since each piece is either ally or not.

Answer 3

Another approach can be implement in every piece class

Public String getColor(){
    return this.color;
}

And to compare...

isAlly(Piece p){
    return this.color.equals(p.getColor());
}

For sure you can use int instead of string to optimize the comparison