0

I'm building a small game similar to chess. I'd like to be able to reuse the structure for another version of checkers too. I'm modeling the games with interfaces (showing only relevant ones):

RuleOrchestrator.class

public interface RuleOrchestrator {
    Collection<Move> allowedMoves(Player player, Move move);
}

Player.class

public interface Player {
    Move getMove(Board board);
}

AIEngine.class

public interface AIEngine {
    Move computeBestMove(Board board, RuleOrchestrator ruleOrchestrator);
}

The general design seems to be fine for both games. However, I find myself doing quite a few castings. After thinking about it, I have come to what looks like the root of the issue: I'm using the interfaces only to define the general workflow of the games, but many times the implementations are not interchangeable. The two implementations of AIEngine work with any Board and RuleOrchestrator, but almost every other concrete class works only with the concrete classes in their own "realm" (game).

Chess.class

public class Chess implements Game {

    private final Player player1;
    private final Player player2;
    private final RuleOrchestrator ruleOrchestrator;
    private final Board board;

    public Chess(String player1, String player2, int size) {
        this.player1 = new HumanPlayer(player1);
        this.player2 = new HumanPlayer(player2);
        this.board = new ChessBoard(size);
        this.ruleOrchestrator = new ChessOrchestrator(board); // 1
    }
    // ...
}

ChessOrchestrator.class

public class ChessOrchestrator implements RuleOrchestrator {
    private final Board board; // 2
    public ChessOrchestrator(Board board) {
        this.board = board;
    }

    @Override
    public Collection<Move> allowedMoves(Player player, Move move) {
        // Do stuff
        ... ((ChessBoard)board).getKing(); // 3
        ChessMove chessMove = (ChessMove) move; // 4
        // More things
        // ...
    }
}

For instance, ChessOrchestrator expects to work with a chess board. It needs chess-specific info and calls methods specific to ChessBoard. It probably doesn't make sense to define the field at // 2 with type Board. Using ChessBoard would avoid having to cast. Then, I would need o either work directly with ChessBoard also in Chess, or casting in line // 1. Furthermore, move also needs to be cast to a ChessMove at // 4.

Not only I dislike these casts, but I'm afraid they're the symptom of a bigger problem. Is this a code smell? How can I avoid these castings and improve the app's design?

  • 1
    What is a "code smell?" – Robert Harvey Aug 31 '17 at 23:37
  • @RoberyHarvey You aren't familiar with code smells? Or were you asking that to see if he was familiar with the proper definition? – Vince Emigh Sep 7 '17 at 18:59
6

How can I avoid these castings and improve the app's design?

This looks like a candidate for use of generic types. A generic base could be written to provide basic workflow, though taking a type argument, meaning that for the Chess family you would supply ChessBoard.

You might have IOrchestrator<BoardType> interface, and therein you can declare methods that operate over BoardType.

You might further implement an abstract generic base class Orchestrator<BoardType> implements IOrchestrator<BoardType>. This abstract class can provide a field board of type BoardType and some default method implementations for the interface.

You might then have ChessOrchestrator extends Orchestrator<ChessBoard>, which can use the field board of BoardType knowing it is a ChessBoard, hence without casting.

  • 3
    I doubt that the solution is generics. It may reduce your casting problems, but IMHO the root cause is that the games are not similar enough to share the same interfaces. – Timothy Truckle Sep 1 '17 at 11:55
  • @TimothyTruckle Why are they not similar enough? Does a chess board have anything other than a specific set of allowed moves? – NickL Oct 12 '17 at 0:32
  • @NickL "Why are they not similar enough?" We might think of some implementation where your board has only generic methods applyable to any game but the OP has this call: ((ChessBoard)board).getKing(); which is not "generic" at all and highly coupled to a game that has "Kings". This means the OPs problem needs to be solven on architecture level. Looking for a "more generic" interface is not enough. – Timothy Truckle Oct 12 '17 at 7:46
  • 1
    @TimothyTruckle you forgot that that call is made in ChessOrchestrator, which is specifically implemented for the chess game, and is not a generic class. If board would be generic, it would be of type ChessBoard for that class. This is exactly what generics is for. – NickL Oct 12 '17 at 8:06
3

Implementations of the same interface that are not interchangeable violate the Liskov Substitution Principle, and indeed in such cases the need to cast is a smell.

In essence, the implementing class violates the interface since it makes additional assumptions about the methods' input arguments, assumptions which are not a part of the interface. The code which uses the RuleOrchestrator interface must pass the right subtype of Board to the allowedMoves method, meaning that it must know something about the concrete implementation class of RuleOrchestrator which isn't expressed in the interface. This means that the consuming code is tightly coupled to the implementation.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.