I'm currently working on a small chess game written in Java (on GitHub). The board is modeled as a Board object with a 2D array of Piece objects :
public class Board {
private final int ROWS = 8;
private final int COLS = 8;
private Piece[][] board;
private List<Move> moveList;
[...]
}
At first, I tried to implement all the board's possible states / legal move generation (isCheck
, isCheckMate
, isStaleMate
, legalMoves
...) inside the Board class.
For example :
private List<Move> moves(Color color) {
List<Move> allMoves = new ArrayList<Move>();
for (int row = 0; row < ROWS; row++) {
for (int col = 0; col < COLS; col++) {
Square src = new Square(row, col);
Piece piece = getPiece(src);
if (piece == null || !piece.isColor(color))
continue;
allMoves.addAll(piece.availableMoves(src, this));
}
}
return allMoves;
}
However it ended up being about 300 lines and it was not very easy to read (especially because of the duplication of the board iteration loops).
So I decided to try another approach : I removed all the state evaluation code and replaced it with this method :
public void accept(BoardVisitor bv) {
for (int row = 0; row < rows; row++) {
for (int col = 0; col < cols; col++) {
bv.visit(board[row][col], new Square(row, col));
}
}
}
I then created a set of classes to "evaluate" the different states :
public class CheckEvaluator implements BoardVisitor {
private Square kingSquare;
private Board board;
private Color color;
private boolean isCheck = false;
public CheckEvaluator(Color color, Board board) {
this.board = board;
this.color = color;
}
@Override
public void visit(Piece piece, Square src) {
isCheck = isCheck || piece.canGoTo(src, kingSquare, board);
}
public boolean getResult() {
this.kingSquare = board.findKing(color);
board.accept(this);
return isCheck;
}
}
I regrouped all these evaluators inside a single class :
public class BoardEvaluator {
private Board board;
public BoardEvaluator(Board board) {
this.board = board;
}
public boolean isCheck(Color color) {
CheckEvaluator ce = new CheckEvaluator(color, board);
return ce.getResult();
}
public boolean isCheckMate(Color color) {
CheckMateEvaluator cme = new CheckMateEvaluator(color, board);
return cme.getResult();
}
public boolean isStaleMate() {
StaleMateEvaluator sme = new StaleMateEvaluator(board);
return sme.getResult();
}
public List<Move> legalMoves(Color color) {
LegalMovesEvaluator lme = new LegalMovesEvaluator(color, board);
return lme.getResult();
}
This version seems clearer and easier to me but I don't have a lot of experience and I'd be very glad to get some feedback about it:
- Do you think this is a valid design?
- Is my BoardVisitor a good (if simple) implementation of the Visitor pattern?
COLS
andROWS
and 2.bv.visit(board[row][col], new Square(col, row));
looks confusing to me. You should consider reordering the parameters and always userow
orcol
as first parameter \$\endgroup\$BoardEvaluator
method is called? Should I try to implement some kind of singleton pattern? (only one instance for each board/color couple...) \$\endgroup\$getResult()
and use the Evaluators as singletons. Thanks \$\endgroup\$