Is this an abuse of the type system?

Question

I'm working on a hobby project of the game Baroque Chess. For those that haven't played it, it has the same basic rules as chess, but the methods for movement and capturing are different.

Naturally I created standard classes for the game: GameState, Board, Square, and a class designated for each piece that inherit from a BasePiece.

Each piece has 2 main virtual methods, GetPossibleMoves(Board board) and GetCapturedSquares(Board board, Square toSquare).

Now, one of the pieces, the Imitator, captures pieces by "imitating" the piece that it captures. For example, a Long Leaper can capture pieces by jumping over them. This means that the Imitator can jump over enemy Long Leapers to capture them (but cannot jump anything else).

I completed the GetCapturedSquares() functionality for all the pieces except the Imitator (which is definitely the trickiest of the pieces).

My basic algorithm for the Imitator was:

• find all the squares with enemy pieces on them
• for each enemy piece...
  • create a simulated piece in the same location as the Imitator
  • find the valid captures if it were the simulated piece moving to a chosen square
  • verify the enemy square is in that list of captured squares

Since I had already written the code for the other pieces' movements, I figured I would just instantiate new pieces and use their GetCapturedSquares() methods depending on which type of piece the enemy was. To do this, I setup up a Dictionary as you can see here that maps a System.Type to an instantiated object of said type:

var typeToPiece = new Dictionary<Type, BasePiece>() 
{
    {typeof(Pincer), new Pincer() { Color = this.Color, CurrentSquare = this.CurrentSquare}},
    {typeof(Withdrawer), new Withdrawer() { Color = this.Color, CurrentSquare = this.CurrentSquare }},
    {typeof(Coordinator), new Coordinator() { Color = this.Color, CurrentSquare = this.CurrentSquare }},
    {typeof(LongLeaper), new LongLeaper() { Color = this.Color, CurrentSquare = this.CurrentSquare }},
    {typeof(King), new King() { Color = this.Color, CurrentSquare = this.CurrentSquare }},
};
//...
var possibleMoves = typeToPiece[enemySquare.Occupant.GetType()].GetPossibleMoves(board, toSquare);

Doing this makes me feel dirty inside. Is it more appropriate to create an enum or string that represents the piece type as the dictionary key, or does it really not matter? Is there a different way to handle this? I am of the opinion that it's fine just the way it is but I am interested in hearing your thoughts.

Answer 1

I think you should add an abstract method to BasePiece that "clones" the current piece and returns the simulated piece.

You'd override this method in every piece type. To share code between them, you can add a protected method to the base class that copies the shared properties to the instance passed to it:

abstract class BasePiece {
   protected BasePiece(BasePiece pieceToClone) {
      this.Color = pieceToClone.Color;
      this.CurrentSquare = pieceToClone.CurrentSquare;
   }
   public abstract BasePiece GetSimulatedClone();
}

class King : BasePiece {
   protected King(King pieceToClone) : base(pieceToClone) { }
   public King() { }
   public override BasePiece GetSimulatedClone() {
       return new King(this);
   }
}

In general, whenever you are switching based on the type of a variable, you should think twice and see if you can polymorphism instead.

Answer 2

This is a little more elegant to my mind:

    private abstract class Piece {}
    private class King : Piece { }
    private class Imitator : Piece { }

    private void main(object sender, EventArgs e) {
        Piece x;
        x = CreateNewPiece(new King());
        x = CreateNewPiece(new Imitator());
    }

    private T CreateNewPiece<T>(T piece) where T : Piece, new() {
        return new T();
    }

It relies on the new() generic constraint to instantiate a type variable.

Answer 3

I personally agree that you're okay to do what you're doing since it's a small hobby project. If you really want to get around the problem you're describing, though, you could create a separate tier of Mover objects that handles the logic surrounding the actual movement of different pieces. Then, rather than asking a piece what moves it can do, you pass that piece's position information, along with the board state information, to the Mover, which tells you what moves that piece could make. The Mover associated with an Imitator could then be a combination of all the other movers in the way you describe, but without the need to create fake "pieces" on the fly.

Another suggestion I'd make, which is more related to logic than to your model, is to change your logic like so:

• for each piece type (or mover type)
  • create a simulated piece in the same location as the Imitator
  • find the valid captures if it were the simulated piece moving to a chosen square
  • only keep the captures where the occupant of the enemy square is of the piece type you're checking

This is a subtle difference, but will significantly reduce the amount of calculation required.

Update

Recursive's comment made me realize I may not have been clear enough about how the Mover tier would work. The idea is to have a KingMover, PincerMover, and so forth, which know about the moves for a specific piece type. Since they're tied to a piece's type rather than the piece itself, they could even be singletons. Each piece type could have a Mover field that points to its mover, and then either your business logic could call that Mover's GetPossibleMoves method directly, or your piece's GetPossibleMoves method could simply call the Mover's method, passing itself in as an argument. The ImitatorMover would know to ask each other type of mover for its possible moves, and then filter those moves based on whether they would be attacking a piece of the type associated with that mover.

You'd have almost the same code as in the current system, but the code for each Piece could really just focus on representing that Piece's information (position, color, etc.), whereas the code for actually determining how a piece moves would be moved into a separate tier of classes. Each class would have a single purpose.

Answer 4

Without touching this specific problem, there's nothing inherently wrong with the idea of a dictionary that lets you look up objects by their type. In fact, .NET FCL provides such a type already - it's called KeyedByTypeCollection<T>. It's probably better for such things, because it ensures that key of an object is its type as returned by GetType() (and not some other random type), and won't let you add two objects of the same type.