C#到GetAvailableMoves中的象棋模型

Question

只是好奇，如果有人可以审查我的设计，并实现以下要求。我是一个中层开发人员，这是一份新工作。我正在寻找改进的方法，如果你是面试官的话，你的总体想法。

代码测试说明：

编写一个简单的应用程序/服务，接受两个输入:棋子(国王、王后、鲁克或主教之一)和使用标准国际象棋表示法(a1、b2、c3等)的起始位置。该服务提供棋盘上所有可能的位置，棋子可以从该位置移动到标准棋盘上。

假设:棋盘上没有其他的棋子，在某些情况下，相对于另一个棋子来说，没有特别的考虑，例如国王和车在一次转弯时一起移动。

评审人员将寻找正确的功能、设计模式的使用、单元测试的使用、面向对象编程的使用、代码重用等。在单个解决方案中，每个阶段都可以用一个新的项目来表示。

第一阶段：

创建一个C# 7 .net核心类库项目，其中包含棋子模型类和确定可能移动的算法。

在一个单独的项目中创建单元测试，并对算法进行足够的覆盖率测试。

其他阶段没有列出，因为我在第一阶段。

我没有使用任何设计模式，因为我认为这是不必要的，这将是过分的编码练习。我应该为规则添加一些类似于策略设计模式的内容吗？我想我可以解释一下，添加设计模式会为这些需求增加不必要的复杂性。

下面的代码-注意:所有其他棋子都类似于女王级。

public class Board
{

    private const int MAX_ROW_SIZE = 8;
    private const int MAX_COLUMN_SIZE = 8;

    public readonly Square[,] squares = new Square[MAX_ROW_SIZE,MAX_COLUMN_SIZE];


    public Board()
    {
        for (int i = 0; i < MAX_ROW_SIZE; i++)
        {
            for (int j = 0; j < MAX_COLUMN_SIZE; j++)
            {
                squares[i,j] = new Square(i, j);
            }
        }
    }


    public List<Square> GetAvailableMoves(Square startingSquare)
    {
        if (startingSquare.piece == null)
        {
            throw new Exception("Square must have a chess piece on it to get the available moves.");
        }

        List<Square> validPieceMoves = new List<Square>();

        for (int i = 0; i < MAX_ROW_SIZE; i++)
        {
            for (int j = 0; j < MAX_COLUMN_SIZE; j++)
            {
                int destColumn = j;
                int destRow = i;
                int sourceColumn = startingSquare.column;
                int sourceRow = startingSquare.row;

                bool isNewSquareValid = startingSquare.piece.IsValidMove(sourceColumn, sourceRow, destColumn, destRow);

                if (isNewSquareValid)
                {
                    Square possibleSquare = new Square(destRow, destColumn);
                    validPieceMoves.Add(possibleSquare);
                }
            }
        }

        return validPieceMoves;
    }
}



public abstract class PieceBase
{

    public readonly PieceType pieceType; 


    public PieceBase(PieceType pieceType)
    {
        this.pieceType = pieceType;
    }


    internal abstract bool IsValidMove(int sourceColumn, int sourceRow, int destColumn, int destRow);


    protected bool _IsDiagonalMove(int sourceColumn, int sourceRow, int destColumn, int destRow)
    {
        bool isDiagonalMove = Math.Abs(destColumn - sourceColumn) == Math.Abs(destRow - sourceRow);
        return isDiagonalMove;
    }

    protected bool _IsVerticalMove(int sourceColumn, int sourceRow, int destColumn, int destRow)
    {
        bool isVerticalMove = sourceColumn == destColumn && sourceRow != destRow;
        return isVerticalMove;
    }

    protected bool _IsHorizontalMove(int sourceColumn, int sourceRow, int destColumn, int destRow)
    {
        bool isHorizontalMove = sourceRow == destRow && sourceColumn != destColumn;
        return isHorizontalMove;
    }
}



public class Square
{

    public readonly int row;
    public readonly int column;

    public PieceBase piece;


    public Square(int row, int column)
    {
        this.row = row;
        this.column = column;
    }

    public Square(int row, int column, PieceBase piece)
    {
        this.row = row;
        this.column = column;
        this.piece = piece;
    }

    public string GetDisplayCoordinates()
    {
        // 0 + 65 is the start of ascii uppercase characters
        // 65 + 32 is the start of ascii lowercase characters
        char rowCoordinate = Convert.ToChar(row + 65 + 32);

        return rowCoordinate + column.ToString();
    }
}



public class Queen : PieceBase
{

    public Queen() : base(PieceType.QUEEN)
    {

    }

    internal override bool IsValidMove(int sourceColumn, int sourceRow, int destColumn, int destRow)
    {
        bool isDiagonalMove = this._IsDiagonalMove(sourceColumn, sourceRow, destColumn, destRow);
        bool isVerticalMove = this._IsVerticalMove(sourceColumn, sourceRow, destColumn, destRow);
        bool isHorizontalMove = this._IsHorizontalMove(sourceColumn, sourceRow, destColumn, destRow);

        return isDiagonalMove || isVerticalMove || isHorizontalMove;
    }
}

下面是NUnit测试

[TestFixture]
public class BoardTests
{

    private Board chessBoard = new Board();


    [Test]
    public void AllAvailableBishopMovesAreValid()
    {
        Square bishopStartSquare = new Square(4, 4, new Bishop());

        List<Square> availableChessMoves = chessBoard.GetAvailableMoves(bishopStartSquare);

        foreach (Square availableMove in availableChessMoves)
        {
            bool isValidMove = this._IsDiagonalMove(bishopStartSquare, availableMove);

            Assert.IsTrue(isValidMove, "Move is not valid for Bishop from " + bishopStartSquare.GetDisplayCoordinates() + " to " + availableMove.GetDisplayCoordinates());
        }
    }

    [Test]
    public void AllAvailableRookMovesAreValid()
    {
        Square rookStartSquare = new Square(4, 4, new Rook());

        List<Square> availableChessMoves = chessBoard.GetAvailableMoves(rookStartSquare);

        foreach (Square availableMove in availableChessMoves)
        {
            bool isValidMove = this._IsVerticalMove(rookStartSquare, availableMove)
                               || this._IsHorizontalMove(rookStartSquare, availableMove);

            Assert.IsTrue(isValidMove, "Move is not valid for Rook from " + rookStartSquare.GetDisplayCoordinates() + " to " + availableMove.GetDisplayCoordinates());
        }
    }

    [Test]
    public void AllAvailableQueenMovesAreValid()
    {
        Square queenStartSquare = new Square(4, 4, new Queen());

        List<Square> availableChessMoves = chessBoard.GetAvailableMoves(queenStartSquare);

        foreach (Square availableMove in availableChessMoves)
        {
            bool isValidMove = this._IsVerticalMove(queenStartSquare, availableMove) 
                               || this._IsHorizontalMove(queenStartSquare, availableMove) 
                               || this._IsDiagonalMove(queenStartSquare, availableMove);

            Assert.IsTrue(isValidMove, "Move is not valid for Queen from " + queenStartSquare.GetDisplayCoordinates() + " to " + availableMove.GetDisplayCoordinates());
        }
    }

    [Test]
    public void AllAvailableKingMovesAreValid()
    {
        Square kingStartSquare = new Square(4, 4, new King());

        List<Square> availableChessMoves = chessBoard.GetAvailableMoves(kingStartSquare);

        foreach (Square availableMove in availableChessMoves)
        {
            bool isValidMove = this._IsVerticalMove(kingStartSquare, availableMove)
                               || this._IsHorizontalMove(kingStartSquare, availableMove)
                               || this._IsDiagonalMove(kingStartSquare, availableMove);

            isValidMove = isValidMove && this._HasMovedOnlyOneSquare(kingStartSquare, availableMove);

            Assert.IsTrue(isValidMove, "Move is not valid for King from " + kingStartSquare.GetDisplayCoordinates() + " to " + availableMove.GetDisplayCoordinates());
        }
    }

    [Test]
    public void StartingSquareHasNoChessPiece()
    {
        Square startingSquare = new Square(1, 0);

        Exception ex = Assert.Throws<Exception>(() => chessBoard.GetAvailableMoves(startingSquare));

        Assert.That(ex.Message == "Square must have a chess piece on it to get the available moves.");
    }


    #region utility methods

    private bool _IsDiagonalMove(Square sourceSquare, Square destSquare)
    {
        int destColumn = destSquare.column;
        int destRow = destSquare.row;

        int sourceColumn = sourceSquare.row;
        int sourceRow = sourceSquare.row;

        bool isDiagonalMove = Math.Abs(destColumn - sourceColumn) == Math.Abs(destRow - sourceRow);
        return isDiagonalMove;
    }

    private bool _IsVerticalMove(Square sourceSquare, Square destSquare)
    {
        int destColumn = destSquare.column;
        int destRow = destSquare.row;

        int sourceColumn = sourceSquare.row;
        int sourceRow = sourceSquare.row;

        bool isVerticalMove = sourceColumn == destColumn && sourceRow != destRow;
        return isVerticalMove;
    }

    private bool _IsHorizontalMove(Square sourceSquare, Square destSquare)
    {
        int destColumn = destSquare.column;
        int destRow = destSquare.row;

        int sourceColumn = sourceSquare.row;
        int sourceRow = sourceSquare.row;

        bool isHorizontalMove = sourceRow == destRow && sourceColumn != destColumn;
        return isHorizontalMove;
    }

    private bool _HasMovedOnlyOneSquare(Square sourceSquare, Square destSquare)
    {
        int destColumn = destSquare.column;
        int destRow = destSquare.row;

        int sourceColumn = sourceSquare.row;
        int sourceRow = sourceSquare.row;

        bool isRowMoveOne = sourceRow - destRow * 1 == 1;
        bool isColumnMoveOne = sourceColumn - destColumn * 1 == 1;

        return isRowMoveOne && isColumnMoveOne;
    }

    #endregion
}

Answer 1

设计

这是一个项目的第一阶段，而且还会有更多的阶段，(大概)新的需求会出现在后面的阶段。

在审查设计时要考虑的一件事是，它对需求中的扩展有多好。

因此，让我们考虑一下这个项目的一些可能的额外需求

1)加入骑士和棋子

2)允许板上的其他部分(这将阻止移动)

那么，我们如何将这些要求纳入当前的设计呢？

目前，确定有效的移动被划分为两个类。

• 棋盘，它要求一块‘你能在这里移动’的每一个位置的董事会。
• 它可以报告它是否能移动到那里。

在当前实现中，用于检查特定类型移动(对角线、水平、垂直)的代码位于基件类中。

添加骑士和典当

我们添加两个新的子类。典当，特别是，带来了当前的形状，检查每个可能的位置和检查它是否有效的问题。一个棋子可以移动到三个地方中的一个，或者四个地方(如果我们忽略其他部分的话，两个中的一个)。检查每一个可能的位置，看看它是否有效似乎非常浪费。骑士有相对较少的可能有效的移动，而这些检查是与现有的对角线，水平，垂直检查无关。

我们可以将支票放在Knight子类中，比如IsKnightMove()。这是可行的，但是基类中的不相关的IsDiagonalMove()、IsVerticalMove()、IsHorizontalMove()例程有点小问题。

检查每一个可能的动作，看它是否有效。

假设我们设置了一个初始棋盘，并希望找到每个棋子的有效动作。在后排的棋子中，只有骑士可以移动，但是我们检查每个棋子的每一个可能的位置，并且没有办法修剪检查，因为可能的移动是独立的。

那么，我们如何才能改变形状来解决一些问题呢？

如果我们让每个部分负责生成一个可以移动的位置列表(传入当前的板状态-目前尚未使用-但我们认为，雅格尼除外，它将是必要的)，我们可以利用我们对这些碎片如何移动的知识来生成可能的移动，并在以后需要时，根据板子上的其他部分调整检查。

public interface IChessPiece
{
    BoardLocation CurrentLocation { get; set; }
    ChessPieceType Type { get; }
    IEnumerable<Move> GetValidMoves(ChessBoard board);
}

public class BoardLocation
{
    private const int BoardSize = 8;

    private static bool IsInRange(int pos)
    {
        return (pos >= 1) && (pos <= BoardSize);
    }

    public BoardLocation(int row, int col)
    {
        if (!IsInRange(row))
        {
            throw new ArgumentOutOfRangeException("row");
        }
        if (!IsInRange(col))
        {
            throw new ArgumentOutOfRangeException("col");
        }
        Row = row;
        Column = col;
    }
    public int Row { get; }
    public int Column { get; }
}

public class Move
{
    public Move(IChessPiece piece, BoardLocation endingLocation)
    {
        Piece = piece ?? throw new ArgumentNullException("piece");
        EndingLocation = endingLocation ?? throw new ArgumentNullException("endingLocation");
    }
    public IChessPiece Piece { get; }
    public BoardLocation EndingLocation { get; }
}

备注：

在编写代码时，我们可能会遇到多少就是足够的问题(YAGNI存在是有原因的)。在上面，板位置只是行和列位置的一个简单容器。它还可以负责格式化显示位置，例如e4，或者我们可以将其留给外部格式化程序，以便我们可以使用旧的表示法(KP4)。它还可以负责解析输入字符串，或者我们可以将其留给另一个类处理。可以省略Move类(只返回一个BoardLocations列表)，但是我们可能想知道片段/移动组合。

在很多情况下，这可以归结为个人偏好问题(MoPP (tm))。

寻找有效的移动

有了这个形状，我们可以使用知识，每一个部分如何移动，以产生可能的移动(可选地使用董事会状态)。d2上的一个棋子现在可以移动到d3或d4 (如果没有任何阻碍)，或者如果有什么东西在那里或者正在经过的话，可能会使用c3或e3。

一个骑士可以产生2到6个可能的移动。

国王、王后、鲁克和主教都可以在各自的职业中产生自己可能的动作。

共享功能？生成迁移/检查移动是否有效的代码可以共享，但不需要在基类中，它可能位于共享实用程序类中(同样是一个MoPP，我不喜欢用不是所有子类使用的代码来杂乱基类)。

public abstract class ChessPiece : IChessPiece
{
    private BoardLocation _currentLocation;
    public BoardLocation CurrentLocation
    {
        get => _currentLocation;
        set => _currentLocation = value ?? throw new ArgumentNullException();
    }
    public abstract ChessPieceType Type { get; }
    public abstract IEnumerable<Move> GetValidMoves(ChessBoard board);
}

public class King : ChessPiece
{
    private readonly static int[][] MoveTemplates = new int[][]
    {
      new [] { 1, -1 },
      new [] { 1, 0 },
      new [] { 1, 1 },
      new [] { 0, -1 },
      new [] { 0, 1 },
      new [] { -1, -1 },
      new [] { -1, 0 },
      new [] { -1, 1 },

    };

    public override ChessPieceType Type => ChessPieceType.King;

    public override IEnumerable<Move> GetValidMoves(ChessBoard board)
    {
        return ChessMoveUtilities.GetMoves(board, this, 1, MoveTemplates);
    }
}

public class Queen : ChessPiece
{
    private readonly static int[][] MoveTemplates = new int[][]
    {
      new [] { 1, -1 },
      new [] { 1, 0 },
      new [] { 1, 1 },
      new [] { 0, -1 },
      new [] { 0, 1 },
      new [] { -1, -1 },
      new [] { -1, 0 },
      new [] { -1, 1 },

    };

    public override ChessPieceType Type => ChessPieceType.King;

    public override IEnumerable<Move> GetValidMoves(ChessBoard board)
    {
        return ChessMoveUtilities.GetMoves(board, this, board.Size, MoveTemplates);
    }
}

internal static class ChessMoveUtilities
{
    private static bool IsValid(ChessBoard board, BoardLocation current, int deltaRow, int deltaCol, out BoardLocation location)
    {
        location = null;
        var newRow = current.Row + deltaRow;
        if ((newRow <= 0) ||(newRow > board.Size)) return false;

        var newCol = current.Column + deltaCol;
        if ((newCol <=0) || (newCol > board.Size)) return false;

        location = new BoardLocation(newRow, newCol);
        return true;
    }

    internal static IEnumerable<Move> GetMoves(ChessBoard board, IChessPiece piece, int range, IEnumerable<int[]> mults)
    {
        if (board == null) throw new ArgumentNullException("board");
        if (piece == null) throw new ArgumentNullException("piece");
        if (range < 1) throw new ArgumentOutOfRangeException("range");
        if (mults == null || !mults.Any()) throw new ArgumentException("mults");

        var ret = new List<Move>();

        foreach( var mult in mults)
        {
            for (var radius = 1; radius <= range; radius++)
            {

                var deltaX = radius * mult[0];
                var deltaY = radius * mult[1];
                if(IsValid(board, piece.CurrentLocation, deltaX, deltaY, out BoardLocation newLocation))
                {
                    ret.Add(new Move (piece, newLocation));
                }
                else
                {
                    break;
                }
            }
        }
        return ret;

    }

}

备注：

可以(目前)用一个以ChessPieceType和MoveTemplates为参数的公共基类的实例替换国王、王后、鲁克和毕晓普的个人类，但了解国际象棋和提前思考，如果我们加入卡斯特灵和“不进入制约”规则，那么国王和鲁克就需要成为单独的类，合并女王和主教似乎有点过火了。

Answer 2

一些快速评论WRT风格：

• 公共属性至少应该有一个{ get; } (IMHO；我不是public readonly的粉丝，我更喜欢{ get; private set; })，应该是PascalCase。
• 不要仅仅抛出任何Exception，抛出正确的一个，例如ArgumentException。
• 下划线只应用于在类范围内的私有属性前缀，当然不应用于方法。
• 不要仅为返回下一行的值而赋值。把这两行合并成一条。
• 不要毫无意义地缩写:没有必要使用"dest“而不是”目的“。这肯定不会使您的代码更清晰。
• 在您的“实用程序”方法中分配destColumn、destRow、sourceColumn和sourceRow有什么意义？为什么不简单地使用destSquare.column等呢？
• 使用串内插而不是串联。