Return to Answer

• 1

• 392
• 1
• 8

• Some of the terminology you're using is unusual. I would say Piece or Man instead of Figure, and "castling" instead of "Rochade". GetValidTurns should probably be called GetValidMoves.
• As Eric Lippert suggests on your other question, you should probably create a struct called BoardPosition which contains a row and a column as ints.
• The code for BoardPosition should contain comments indicating how the ranks and files are numbered. Maybe the constructor should check that the given numbers are in range.
• It would be useful for BoardPosition to have a method public BoardPosition Move(int right, int up) which returns a new BoardPosition with the coordinates altered appropriately.
• If you wanted, you could make GetValidTurns on QueenPiece just a single line: return GetRookMoves().Concat(GetBishopMoves()).ToList();
• The methods GetValidTurns on RookPiece and GetRookMoves on QueenPiece are very similar. Consider replacing these two methods with a single GetRookMoves method. You might have this method as a protected method in the Figure class, or perhaps as a static method in a new static class, called something like ChessMoves or (Eric's suggestion) Rulebook.
• In GetRookMoves, the while-loops for rightMove, topMove, leftMove, and downMove are all very similar. See if you can combine these four pieces of code into one.
• I find the method call WillCollideWithEnemy(rightMove, PieceColor).Item1 to be confusing. Given the name WillCollideWithEnemy, I would expect that method to return a bool, but a bool doesn't have an Item1. Consider doing the following instead:
• Create a method public Figure GetEnemyAt(boardPosition, pieceColor), which returns null if the square at boardPosition is empty or contains an allied piece.
• Create a separate method public bool ContainsEnemy(boardPosition, pieceColor), which merely checks if the square contains an enemy or not and returns true or false. (This method could be implemented as a one-liner: return (GetEnemyAt(boardPosition, pieceColor) != null);.)
• For the while-loops in GetRookMoves, the logic seems a little more complicated than necessary.
• The startingMoves variable isn't necessary. Remove it, and initialize validMoves as the empty list.
• The implementation of the while-loops could be something like the following:

—

BoardPosition destination = CurrentPosition;
while(true)
{
 destination = destination.Move(right: 1, up: 0);
 if (IsOutOfBounds(destination) || WillCollideWithAlly(destination, PieceColor))
 {
 break;
 }
 validMoves.Add(destination);
 if (ContainsEnemy(destination, PieceColor))
 {
 break;
 }
}

• Consider making all of the members of ImagePaths const instead of static. You should be able to use definitions such as public const string BlackPawnImagePath = assetsPath + @"Black\b-peshka.png";. (Making ImagePaths a static class was a good idea.)
• Consider making Rochade a static class and turning all of its non-constant properties (including newKingMove) into parameters and/or local variables of the methods.
• In the PassedTurns class, shouldn't Actions, Positions, and PieceTypes all be lists?