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.