6
\$\begingroup\$

Intro

This post continues the A chess engine in Java: generating white pawn moves.

I was advised to choose between efficiency and type safety. Since this is my first attempt at a chess engine, I have decided that I will stick to type safety this time.

In this post, I essentially extracted the move generating logic to a dedicated class implementing an expansion interface. That way, my code will stay well modularized.

(The entire repository is here.)

Code

com.github.coderodde.game.chess.ChessBoardState.java:

package com.github.coderodde.game.chess;

import com.github.coderodde.game.chess.impl.WhitePawnExpander;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

/**
 * This class implements a chess board state.
 * 
 * @version 1.0.1 (Jun 26, 2024)
 * @since 1.0.0 (Jun 22, 2024)
 */
public final class ChessBoardState {
    
    public static final int N = 8;
    
    private Piece[][] state;
    private boolean[] whiteIsPreviouslyDoubleMoved = new boolean[N];
    private boolean[] blackIsPreviouslyDoubleMoved = new boolean[N];
    private byte enPassantFlags;
    
    public ChessBoardState() {
        state = new Piece[N][N];
        
        // Black pieces:
        state[0][0] = new Piece(PieceColor.BLACK, PieceType.ROOK, null);
        state[0][7] = new Piece(PieceColor.BLACK, PieceType.ROOK, null);
  
        state[0][1] = new Piece(PieceColor.BLACK, PieceType.KNIGHT, null);
        state[0][6] = new Piece(PieceColor.BLACK, PieceType.KNIGHT, null);
        
        state[0][2] = new Piece(PieceColor.BLACK, PieceType.BISHOP, null);
        state[0][5] = new Piece(PieceColor.BLACK, PieceType.BISHOP, null);
  
        state[0][3] = new Piece(PieceColor.BLACK, PieceType.QUEEN, null);
        state[0][4] = new Piece(PieceColor.BLACK, PieceType.KING, null);
        
        for (int file = 0; file < N; file++) {
            state[1][file] = new Piece(PieceColor.BLACK,
                                       PieceType.PAWN,
                                       new WhitePawnExpander());
        }
        
        // White pieces:
        state[7][0] = new Piece(PieceColor.WHITE, PieceType.ROOK, null);
        state[7][7] = new Piece(PieceColor.WHITE, PieceType.ROOK, null);
  
        state[7][1] = new Piece(PieceColor.WHITE, PieceType.KNIGHT, null);
        state[7][6] = new Piece(PieceColor.WHITE, PieceType.KNIGHT, null);
        
        state[7][2] = new Piece(PieceColor.WHITE, PieceType.BISHOP, null);
        state[7][5] = new Piece(PieceColor.WHITE, PieceType.BISHOP, null);
        
        state[7][3] = new Piece(PieceColor.WHITE, PieceType.QUEEN, null);
        state[7][4] = new Piece(PieceColor.WHITE, PieceType.KING, null);
        
        for (int file = 0; file < N; file++) {
            state[6][file] = new Piece(PieceColor.WHITE,
                                       PieceType.PAWN,
                                       null);
        }
    }
    
    public ChessBoardState(final ChessBoardState copy) {
        this.state = new Piece[N][N];
        
        for (int rank = 0; rank < N; rank++) {
            for (int file = 0; file < N; file++) {
                if (copy.state[rank][file] == null) {
                    continue;
                }
                
                this.state[rank][file] = new Piece(copy.state[rank][file]);
            }
        }
        
        // TODO: Just set?
        System.arraycopy(this.whiteIsPreviouslyDoubleMoved, 
                         0, 
                         copy.whiteIsPreviouslyDoubleMoved, 
                         0, 
                         N);
        
        System.arraycopy(this.blackIsPreviouslyDoubleMoved, 
                         0, 
                         copy.blackIsPreviouslyDoubleMoved, 
                         0, 
                         N);
    }
    
    @Override
    public boolean equals(final Object o) {
        if (!(o instanceof ChessBoardState)) {
            return false;
        }
        
        final ChessBoardState other = (ChessBoardState) o;
        return Arrays.deepEquals(state, other.state);
    }
    
    @Override
    public int hashCode() {
        return Arrays.deepHashCode(state);
    }
    
    /**
     * Clears the entire board. Used in unit testing.
     */
    public void clear() {
        this.state = new Piece[N][N];
    }
    
    /**
     * Returns the piece value at rank {@code rank}, file {@code file}. Used in 
     * unit testing.
     * 
     * @param file the file of the requested piece.
     * @param rank the rank of the requested piece.
     * 
     * @return the piece.
     */
    public Piece get(final int file, final int rank) {
        return state[rank][file];
    }
    
    /**
     * Sets the piece {@code piece} at rank {@code rank}, file {@code file}. Used in 
     * unit testing.
     * 
     * @param file the file of the requested piece.
     * @param rank the rank of the requested piece.
     * @param piece the piece to set.
     */
    public void set(final int file, 
                    final int rank, 
                    final Piece piece) {
        state[rank][file] = piece;
    }
    
    /**
     * Clears the position at rank {@code rank} and file {@code file}. Used in 
     * unit testing.
     * 
     * @param file the file of the requested piece.
     * @param rank the rank of the requested piece.
     */
    public void clear(final int file, final int rank) {
        state[rank][file] = null;
    }
    
    /**
     * Returns the array of previous double moves flags. Used in unit testing.
     * 
     * @return the array of previous double moves flags for the white player. 
     */
    public boolean[] getWhiteIsPreviouslyDoubleMoved() {
        return whiteIsPreviouslyDoubleMoved;
    }
    
    /**
     * Returns the array of previous double moves flags. Used in unit testing.
     * 
     * @return the array of previous double moves flags for the black player. 
     */
    public boolean[] getBlackIsPreviouslyDoubleMoved() {
        return blackIsPreviouslyDoubleMoved;
    }
    
    /**
     * Returns a simple tefiletual representation of this state. Not verrank readable.
     * 
     * @return a tefiletual representation of this state.
     */
    @Override
    public String toString() {
        final StringBuilder stringBuilder =
                new StringBuilder((N + 3) * (N + 2));
        
        int rankNumber = 8;
        
        for (int rank = 0; rank < N; rank++) {
            for (int file = -1; file < N; file++) {
                if (file == -1) {
                    stringBuilder.append(rankNumber--).append(' ');
                } else {
                    final Piece piece = state[rank][file];
                    
                    stringBuilder.append(
                            (piece == null ? 
                                    ((file + rank) % 2 == 0 ? "." : "#") :
                                    piece));
                }
            }
            
            stringBuilder.append('\n');
        }
        
        stringBuilder.append("\n  abcdefgh");
        return stringBuilder.toString();
    }
    
    
    /**
     * Marks that the white pawn at file {@code file} made an initial double move.
     * Used for unit testing.
     * 
     * @param file the file number of the target white pawn. 
     */
    public void markWhitePawnInitialDoubleMove(final int file) {
        this.whiteIsPreviouslyDoubleMoved[file] = true;
    }
    
    /**
     * Marks that the black pawn at file {@code file} made an initial double move.
     * Used for unit testing.
     * 
     * @param file the file number of the target white pawn. 
     */
    public void markBlackPawnInitialDoubleMove(final int file) {
        this.blackIsPreviouslyDoubleMoved[file] = true;
    }
   
    public CellType getCellColor(final int file, final int rank) {
        final Piece piece = state[rank][file];
        
        if (piece == null) {
            return CellType.EMPTY;
        }
        
        if ((piece.getPieceCodeBits() & Piece.WHITE_COLOR) != 0) {
            return CellType.WHITE;
        }
        
        if ((piece.getPieceCodeBits() & Piece.BLACK_COLOR) != 0) {
            return CellType.BLACK;
        }
        
        throw new IllegalStateException("Unknown cell color: " + piece);
    }
    
    public List<ChessBoardState> expand(final PlayerTurn playerTurn) {
        
        final List<ChessBoardState> children = new ArrayList<>();
        
        if (null == playerTurn) {
            throw new NullPointerException("playerTurn is null.");
        } else switch (playerTurn) {
            case WHITE -> {
                for (int rank = 0; rank < N; rank++) {
                    for (int file = 0; file < N; file++) {
                        final CellType cellType = getCellColor(file, rank);
                        
                        if (cellType == CellType.WHITE) {
                            children.addAll(
                                    state[rank]
                                            [file].expand(this, file, rank));
                        }
                    }
                }   
            }
                
            case BLACK -> throw new UnsupportedOperationException();
            
            default -> throw new EnumConstantNotPresentException(
                        PlayerTurn.class,
                        playerTurn.name());
        }
        
        return children;
    }
}

com.github.coderodde.game.chess.WhitePawnExpander.java:

package com.github.coderodde.game.chess.impl;

import com.github.coderodde.game.chess.CellType;
import com.github.coderodde.game.chess.ChessBoardState;
import static com.github.coderodde.game.chess.ChessBoardState.N;
import com.github.coderodde.game.chess.AbstractChessBoardStateExpander;
import com.github.coderodde.game.chess.Piece;
import com.github.coderodde.game.chess.PieceColor;
import com.github.coderodde.game.chess.PieceType;
import java.util.List;

/**
 * This class implements an expander for generating all white pawn moves.
 * 
 * @version 1.0.0 (Jun 26, 2024)
 * @since 1.0.0 (Jun 26, 2024)
 */
public final class WhitePawnExpander extends AbstractChessBoardStateExpander {

    public static final int INITIAL_WHITE_PAWN_RANK = 6;
    public static final int INITIAL_WHITE_PAWN_MOVE_1_RANK = 5;
    public static final int INITIAL_WHITE_PAWN_MOVE_2_RANK = 4;
    public static final int EN_PASSANT_SOURCE_RANK = 3;
    public static final int EN_PASSANT_TARGET_RANK = 2;
    public static final int PROMOTION_SOURCE_RANK = 1;
    public static final int PROMOTION_TARGET_RANK = 0;
    
    @Override
    public void expand(final ChessBoardState root, 
                       final Piece piece,
                       final int file,
                       final int rank,
                       final List<ChessBoardState> children) {
        
        if (rank == INITIAL_WHITE_PAWN_RANK 
                && root.get(file, INITIAL_WHITE_PAWN_MOVE_1_RANK) == null
                && root.get(file, INITIAL_WHITE_PAWN_MOVE_2_RANK) == null) {
            
            // Once here, we can move a white pawn two moves forward:
            final ChessBoardState child = new ChessBoardState(root);
            
            child.markWhitePawnInitialDoubleMove(file);
            
            child.clear(file, INITIAL_WHITE_PAWN_RANK);
            child.set(file, INITIAL_WHITE_PAWN_MOVE_2_RANK, piece);
            children.add(child);
            
            tryBasicMoveForward(root, 
                                children, 
                                file, 
                                rank, 
                                piece);
            return;
            
        } else if (rank == EN_PASSANT_SOURCE_RANK) {
            
            if (file > 0) {
                // Try en passant to the left:
                tryEnPassantToLeft(root, 
                                   piece, 
                                   file, 
                                   children);
            }
            
            if (file < N - 1) {
                // Try en passant to the right:
                tryEnPassantToRight(root,
                                    piece, 
                                    file,
                                    children);
            }
            
            tryBasicMoveForward(root,
                                children, 
                                file, 
                                rank, 
                                piece);
            return;
            
        } else if (rank == PROMOTION_SOURCE_RANK) {
            if (file > 0 && 
                root.getCellColor(file - 1,
                                  PROMOTION_TARGET_RANK) == CellType.BLACK) {
                
                // Once here, can capture to the left and promote:
                for (final PieceType pieceType : PROMOTION_PIECE_TYPES) {
                    final Piece newPiece = 
                            new Piece(
                                    PieceColor.WHITE,
                                    pieceType,
                                    this);
                    
                    final ChessBoardState child = new ChessBoardState(root);
                    
                    child.set(file - 1, PROMOTION_TARGET_RANK, newPiece);
                    child.clear(file, PROMOTION_SOURCE_RANK);
                    children.add(child);
                }
            }
            
            if (file < N - 1 &&
                root.getCellColor(file + 1,
                                  PROMOTION_TARGET_RANK) == CellType.BLACK) {
                
                // Once here, can capture to the right and promote:
                for (final PieceType pieceType : PROMOTION_PIECE_TYPES) {
                    final Piece newPiece = 
                            new Piece(
                                    PieceColor.WHITE,
                                    pieceType,
                                    this);
                    
                    final ChessBoardState child = new ChessBoardState(root);
                    
                    child.set(file + 1, PROMOTION_TARGET_RANK, newPiece);
                    child.clear(file, PROMOTION_SOURCE_RANK);
                    children.add(child);
                }
            }
            
            if (root.getCellColor(file, 
                                  PROMOTION_TARGET_RANK) == CellType.EMPTY) {
                
                // Once here, can move forward an promote:
                for (final PieceType pieceType : PROMOTION_PIECE_TYPES) {
                    final Piece newPiece = 
                            new Piece(
                                    PieceColor.WHITE,
                                    pieceType,
                                    this);
                    
                    final ChessBoardState child = new ChessBoardState(root);
                    
                    child.set(file, PROMOTION_TARGET_RANK, newPiece);
                    child.clear(file, PROMOTION_SOURCE_RANK);
                    children.add(child);
                }
            }
            
            return;
        }
        
        // Try move forward:
        tryBasicMoveForward(root, 
                            children, 
                            file, 
                            rank, 
                            piece);
        
        // Try capture to left:
        if (file > 0 
                && rank > 1 
                && root.getCellColor(file - 1, rank - 1) 
                == CellType.BLACK) {
            
            final ChessBoardState child = new ChessBoardState(root);
            
            child.set(file, rank, null);
            child.set(file - 1, rank - 1, piece);
            
            children.add(child);
        }
        
        // Try capture to right:
        if (file < N - 1
                && rank > 1
                && root.getCellColor(file + 1, rank - 1)
                == CellType.BLACK) {
            
            final ChessBoardState child = new ChessBoardState(root);
            
            child.set(file, rank, null);
            child.set(file + 1, rank - 1, piece);
            
            children.add(child);
        }
    }
    
    private void tryBasicMoveForward(final ChessBoardState root,
                                     final List<ChessBoardState> children,
                                     final int file,
                                     final int rank,
                                     final Piece piece) {
        if (rank > 1 && 
            root.getCellColor(file, rank - 1) == CellType.EMPTY) {
            
            final ChessBoardState child = new ChessBoardState(root);
            child.set(file, rank - 1, piece);
            child.set(file, rank, null);
            children.add(child);
        }
    }
    
    private void tryEnPassantToLeft(final ChessBoardState root,
                                    final Piece piece, 
                                    final int file,
                                    final List<ChessBoardState> children) {
        
        if (!root.getBlackIsPreviouslyDoubleMoved()[file - 1]) {
            return;
        }
        
        final ChessBoardState child = new ChessBoardState(root);
        
        child.clear(file, EN_PASSANT_SOURCE_RANK);
        child.clear(file - 1, EN_PASSANT_SOURCE_RANK);
        child.set(file - 1, EN_PASSANT_TARGET_RANK, piece);
        
        children.add(child);
    }
    
    private void tryEnPassantToRight(final ChessBoardState root,
                                     final Piece piece, 
                                     final int file,
                                     final List<ChessBoardState> children) {
        if (!root.getBlackIsPreviouslyDoubleMoved()[file + 1]) {
            return;
        }
        
        final ChessBoardState child = new ChessBoardState(root);
        
        child.clear(file, EN_PASSANT_SOURCE_RANK);
        child.clear(file + 1, EN_PASSANT_SOURCE_RANK);
        child.set(file + 1, EN_PASSANT_TARGET_RANK, piece);
        
        children.add(child);
    }
}

com.github.coderodde.game.chess.WhitePawnExpanderTest.java:

package com.github.coderodde.game.chess.impl;

import com.github.coderodde.game.chess.AbstractChessBoardStateExpander;
import com.github.coderodde.game.chess.ChessBoardState;
import com.github.coderodde.game.chess.Piece;
import static com.github.coderodde.game.chess.PieceColor.BLACK;
import static com.github.coderodde.game.chess.PieceColor.WHITE;
import com.github.coderodde.game.chess.PieceType;
import static com.github.coderodde.game.chess.PieceType.BISHOP;
import static com.github.coderodde.game.chess.PieceType.KNIGHT;
import static com.github.coderodde.game.chess.PieceType.PAWN;
import static com.github.coderodde.game.chess.PieceType.QUEEN;
import static com.github.coderodde.game.chess.PieceType.ROOK;
import com.github.coderodde.game.chess.PlayerTurn;
import static com.github.coderodde.game.chess.impl.WhitePawnExpander.EN_PASSANT_SOURCE_RANK;
import static com.github.coderodde.game.chess.impl.WhitePawnExpander.EN_PASSANT_TARGET_RANK;
import static com.github.coderodde.game.chess.impl.WhitePawnExpander.INITIAL_WHITE_PAWN_MOVE_1_RANK;
import static com.github.coderodde.game.chess.impl.WhitePawnExpander.INITIAL_WHITE_PAWN_MOVE_2_RANK;
import static com.github.coderodde.game.chess.impl.WhitePawnExpander.INITIAL_WHITE_PAWN_RANK;
import static com.github.coderodde.game.chess.impl.WhitePawnExpander.PROMOTION_SOURCE_RANK;
import static com.github.coderodde.game.chess.impl.WhitePawnExpander.PROMOTION_TARGET_RANK;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import org.junit.Before;
import org.junit.Test;
import static org.junit.Assert.*;

public final class WhitePawnExpanderTest {
    
    private ChessBoardState state;
    private final AbstractChessBoardStateExpander expander = 
            new WhitePawnExpander();
    
    @Before
    public void before() {
        state = new ChessBoardState();
        state.clear();
    }
    
    @Test
    public void moveWhitePawnInitialDoubleMove() {
        state.set(0, INITIAL_WHITE_PAWN_RANK, new Piece(WHITE, PAWN, expander));
        
        final List<ChessBoardState> children = state.expand(PlayerTurn.WHITE);
        
        assertEquals(2, children.size());
        
        final ChessBoardState move1 = new ChessBoardState();
        final ChessBoardState move2 = new ChessBoardState();
        
        move1.clear();
        move2.clear();
        
        move1.set(0, 
                  INITIAL_WHITE_PAWN_MOVE_1_RANK, 
                  new Piece(WHITE, PAWN, expander));
        
        move2.set(0, 
                  INITIAL_WHITE_PAWN_MOVE_2_RANK,
                  new Piece(WHITE, PAWN, expander));
        
        assertTrue(children.contains(move1));
        assertTrue(children.contains(move2));
    }
    
    @Test
    public void whitePawnCannotMoveForward() {
        state.set(4, 5, new Piece(WHITE, PAWN, expander));
        state.set(4, 4, new Piece(BLACK, ROOK, expander));
        
        final List<ChessBoardState> children = state.expand(PlayerTurn.WHITE);
        
        assertTrue(children.isEmpty());
    }
    
    @Test
    public void whitePawnCanEatBothDirectionsAndMoveForward() {
        state.set(5, 4, new Piece(WHITE, PAWN, expander));
        state.set(4, 3, new Piece(BLACK, KNIGHT, expander));
        state.set(6, 3, new Piece(BLACK, ROOK, expander));
        
        final List<ChessBoardState> children = state.expand(PlayerTurn.WHITE);
        
        assertEquals(3, children.size());
        
        final ChessBoardState move1 = new ChessBoardState();
        final ChessBoardState move2 = new ChessBoardState();
        final ChessBoardState move3 = new ChessBoardState();
        
        move1.clear();
        move2.clear();
        move3.clear();
        
        // Capture to the left:
        move1.set(4, 3, new Piece(WHITE, PAWN, expander));
        move1.set(6, 3, new Piece(BLACK, ROOK, expander));
        
        // Move forward:
        move2.set(5, 3, new Piece(WHITE, PAWN, expander));
        move2.set(4, 3, new Piece(BLACK, KNIGHT, expander));
        move2.set(6, 3, new Piece(BLACK, ROOK, expander));
        
        // Caupture to the right:
        move3.set(6, 3, new Piece(WHITE, PAWN, expander));
        move3.set(4, 3, new Piece(BLACK, KNIGHT, expander));
        
        assertTrue(children.contains(move1));
        assertTrue(children.contains(move2));
        assertTrue(children.contains(move3));
    }
    
    @Test
    public void whitePawnCannotMakeFirstDoubleMoveDueToObstruction() {
        state.set(6, 6, new Piece(WHITE, PAWN, expander));
        state.set(6, 5, new Piece(BLACK, BISHOP, expander));
        
        assertTrue(state.expand(PlayerTurn.WHITE).isEmpty());
        
        state.clear();
        state.set(4, 6, new Piece(WHITE, PAWN, expander));
        state.set(4, 5, new Piece(BLACK, ROOK, expander));
        
        assertTrue(state.expand(PlayerTurn.WHITE).isEmpty());
    }
    
    @Test
    public void whitePawnPromotion() {
        state.set(3, PROMOTION_SOURCE_RANK, new Piece(WHITE, PAWN, expander));
        final List<ChessBoardState> children = state.expand(PlayerTurn.WHITE);
        
        assertEquals(4, children.size());
        
        final ChessBoardState move1 = new ChessBoardState();
        final ChessBoardState move2 = new ChessBoardState();
        final ChessBoardState move3 = new ChessBoardState();
        final ChessBoardState move4 = new ChessBoardState();
        
        move1.clear();
        move2.clear();
        move3.clear();
        move4.clear();
        
        move1.set(3, PROMOTION_TARGET_RANK, new Piece(WHITE, QUEEN));
        move2.set(3, PROMOTION_TARGET_RANK, new Piece(WHITE, ROOK));
        move3.set(3, PROMOTION_TARGET_RANK, new Piece(WHITE, KNIGHT));
        move4.set(3, PROMOTION_TARGET_RANK, new Piece(WHITE, BISHOP));
       
        assertTrue(children.contains(move1));
        assertTrue(children.contains(move1));
        assertTrue(children.contains(move1));
        assertTrue(children.contains(move1));
        
        final Set<ChessBoardState> stateSet = new HashSet<>();
        
        stateSet.addAll(Arrays.asList(move1, move2, move3, move4));
        
        assertEquals(4, stateSet.size());
    }
    
    @Test
    public void whitePawnPromotionCaptureBoth() {
        state.set(5, PROMOTION_SOURCE_RANK, new Piece(WHITE, PAWN, expander));
        state.set(4, PROMOTION_TARGET_RANK, new Piece(BLACK, BISHOP));
        state.set(6, PROMOTION_TARGET_RANK, new Piece(BLACK, PAWN));
        
        final List<ChessBoardState> children = state.expand(PlayerTurn.WHITE);
        
        assertEquals(12, children.size());
        
        final ChessBoardState move1 = new ChessBoardState();
        final ChessBoardState move2 = new ChessBoardState();
        final ChessBoardState move3 = new ChessBoardState();
        
        move1.clear();
        move2.clear();
        move3.clear();
        
        // Promote forward:
        move1.set(4, PROMOTION_TARGET_RANK, new Piece(BLACK, BISHOP));
        move1.set(6, PROMOTION_TARGET_RANK, new Piece(BLACK, PAWN));
        
        for (final PieceType pieceType :
                AbstractChessBoardStateExpander.PROMOTION_PIECE_TYPES) {
            
            move1.set(5, PROMOTION_TARGET_RANK, new Piece(WHITE, pieceType));
            assertTrue(children.contains(move1));
        }
        
        // Promote left:
        move2.set(6, 0, new Piece(BLACK, PAWN));
        
        for (final PieceType pieceType :
                AbstractChessBoardStateExpander.PROMOTION_PIECE_TYPES) {
            
            move2.set(4, PROMOTION_TARGET_RANK, new Piece(WHITE, pieceType));
            assertTrue(children.contains(move2));
        }
        
        // Promote right:
        move3.set(4, 0, new Piece(BLACK, BISHOP));
        
        for (final PieceType pieceType :
                AbstractChessBoardStateExpander.PROMOTION_PIECE_TYPES) {
            
            move3.set(6, PROMOTION_TARGET_RANK  , new Piece(WHITE, pieceType));
            assertTrue(children.contains(move3));
        }
    }
    
    @Test
    public void whitePawnEnPassantToLeft() {
        state.set(0, EN_PASSANT_SOURCE_RANK, new Piece(BLACK, PAWN));
        state.set(1, EN_PASSANT_TARGET_RANK, new Piece(BLACK, ROOK));
        state.set(1, EN_PASSANT_SOURCE_RANK, new Piece(WHITE, PAWN, expander));
        
        state.markBlackPawnInitialDoubleMove(0);
        
        final List<ChessBoardState> children = state.expand(PlayerTurn.WHITE);
        
        assertEquals(1, children.size());
        
        final ChessBoardState move = new ChessBoardState();
        
        move.clear();
        move.set(1, EN_PASSANT_TARGET_RANK, new Piece(BLACK, ROOK));
        move.set(0, EN_PASSANT_TARGET_RANK, new Piece(WHITE, PAWN));
        
        assertTrue(children.contains(move));
    }
    
    @Test
    public void whitePawnEnPassantToRight() {
        state.set(3, EN_PASSANT_SOURCE_RANK, new Piece(WHITE, PAWN, expander));
        state.set(4, EN_PASSANT_SOURCE_RANK, new Piece(BLACK, PAWN));
        state.set(3, EN_PASSANT_TARGET_RANK, new Piece(BLACK, ROOK));
        
        state.markBlackPawnInitialDoubleMove(4);
        
        final List<ChessBoardState> children = state.expand(PlayerTurn.WHITE);
        
        assertEquals(1, children.size());
        
        final ChessBoardState move = new ChessBoardState();
        
        move.clear();
        
        move.set(3, EN_PASSANT_TARGET_RANK, new Piece(BLACK, ROOK));
        move.set(4, EN_PASSANT_TARGET_RANK, new Piece(WHITE, PAWN));
        
        boolean pass = children.contains(move);
        
        assertTrue(pass);
    }
}
\$\endgroup\$

3 Answers 3

Reset to default
9
\$\begingroup\$

No offense, but it got worse :) Everything is quite over engineered.

It's not part of the posted code, but combining the enums with the bit masks is pointless. Do one or the other, not both.

You have at least four different places where colors are defined (PieceColor, CellType, PlayerTurn and the constants inside Piece.) There only should be one.

Example: getCellColor (which BTW, it a terrible name. It returns the color of the piece, not the square.) should simply be something like:

Color getPieceColorAt(final int file, final int rank) {
    final Piece piece = state[rank][file];
    return piece == null ? null : piece.getPieceColor();
}

(If it's needed at all. I doubt that it is.)

AbstractChessBoardStateExpander doesn't belong in Piece - or at least shouldn't be 'variable'.

\$\endgroup\$
8
\$\begingroup\$

Coupling

I essentially extracted the move generating logic to a dedicated class implementing an expansion interface. That way, my code will stay well modularized.

Modularizing the logic is a laudable goal, but it doesn't seem to be accomplished, judging by the code shared in the post and in the repository.

Because there is a lot of coupling and there are components with responsibilities which they are not supposed to have.

ChessBoardState de facto isn't really a state, a dumb representation of the chess board at a certain game turn, but rather a "sentient" component. It knows how to expand (it has expand() method for this purpose) and it has a knowledge of the expanders. Basically, ChessBoardState is a game engine (or an element of a game engine), not a state.

Another example of coupling is Piece depending on the expander. This coupling restricts the design without brining any benefit (elaborated further in the example below).

Missing domain-specific names and behavior

Such notions as moving a piece, capturing an enemy's piece, etc. are completely missing.

The absence of methods representing actions on a chess board results in sequences of low-level calls child.clear() + child.set() (instead of move()), child.set() + child.set() (instead of capture()).

Example

Let me give an example of a modular design.

  • Board

We need a dumb class encapsulating the state of a board.

One of the ways to represent the positions of pieces is to use a 64-element byte array new byte[64] (not a nested array) with zeros denoting empty cells. A set of 6 negative and 6 positive values designated to describe pieces of both colors.

I'll talk about piece being a class and enum in a moment

And as I already mentioned this class should expose domain-specific methods: move(), capture(), castle(), etc.

I'll also advise shortening the name, ChessBoard or simply a Board can successfully communicate the same intent. But that's a minor point.

  • Pieces

By describing a chess piece with a class or a record, we cannot leverage object-orientation's most powerful weapon: defining behavior.

Let me explain why introducing methods describing a move in the Piece class is not a good idea.

Note: when I'm saying "moving a piece", it only implies applying supplied coordinates, pieces and board are dumb, i.e. they don't know which move is better than the other, they only know which moves are valid

Implementing a move() method in the Piece class will require awareness of the positions of other pieces, which can be obtained only by querying the Board instance. I.e. we will have to use composition to give each piece a reference to the board. And the board in turn will be still coupled to pieces through aggregation to describe the current state of the game.

Furthermore, while making a move a piece should not only update its own state but also change the state of the board. For that matter the board should expose behavior to ensure that locations of pieces as well data data related to some peculiar cases (enparsant, castling) is properly updated.

By the way, implementation of castling in such a settings when the move is initiated by king piece will be quite convoluted: king.move() -> calls board.castle() -> which triggers the call rook.move(). It'll be much clearner to have only board.castle() instead.

Basically, we ended up with the logic of moves being split between two tightly coupled components.

I.e. Piece will be encroaching on the area of responsibility of the Board. And we end up with a tight coupling between the Board and Piece classes since in order to make a valid move, a piece will require querying the board to gain awareness of how other peaces are located and subsequently updating its own state and the state of the board. Functionality describing moves will be split into two parts: apart from a method describing a move in the Piece class, we will need methods facilitating the move inside the Board class to ensure that positions of pieces are updated and as well as other elements of the board state.

That's error-prone. We should avoid introducing accidental coupling because it creates accidental complexity, i.e. complexity which stems solely from the design of the code, and unrelated to the complexity of the domain.

The idea of a smart chess piece having a knowledge of the expander is the tail wagging the dog. If it was not intended to be smart and serve only as a means of accessing an expander corresponding to a particular piece type (and was not supposed to use the expander), there are still some problems.

Board needs to know how to instantiate Piece. And its constructor expects expander as parameter, which means we failed to decouple the board and the expander implementations.

Furthermore, what's the difference between two instances of Piece with the same type and color? There's none, then why spawning new instances which do not differ from each other?

We can use an enum containing 12 constants can be used to describe all chess pieces of both colors (WHITE_QUEEN, WHITE_KNIGHT, etc.) instead of a class. But in the absence of enum attributes (which will be the case if we move expanders into game engine part), this enum would not be match different from a set of public static final byte fields.

So I suggest implementing the Board as a dumb class maintaining an array representing the current state, turn count and indication which player should make next turn. Instead of exposing get/set style methods, I suggest composing the class API from methods describing the board actions such as move(), capture(), castle(), etc. (these actions might still operate with rank and file as parameters, it's not necessary to expose the internal one-dimensional representation).

As an alternative to two boolean arrays whiteIsPreviouslyDoubleMoved, blackIsPreviouslyDoubleMoved you can store the last move (since the opportunity of en passant can be used only on the preceding turn), or keep a link to the previous board state. Or even you can implement internally Board as a chain of states, a Linked List, containing the history of all moves from the first to the last (which will allow to review the game after it is finished).

One important note: don't mix the concerns of the UI and the game logic. If you're going to develop a command-line or Swing UI, my suggestion is to create a separate representation of the board and chess pieces for that purpose. You can leverage fancy Unicode symbols there ♔ ♕ ♖ ♗ ♘ ♙ ♚ ♛ ♜ ♝ ♞ ♟, but don't pour any of the UI-related requirements into the domain logic.

  • Game engine

The shared expander implementation WhitePawnExpander doesn't tell match about the mechanics of the game engine.

The only thing it conveys: you're going to generate a new board for every valid move of the current player. No clues how these boards will get evaluated and how deep you're going to go (i.e. are you going to simulate several turns?). These missing parts are important for taking design decisions.

So there are several abstractions that might be introduced:

  • GameEngine - maintains, the current state of the board and orchestrating the generation of the next move by the means of Expander and component concerned with selecting the optimal action based on the states produced by the expander.
  • To represent the next move, you would probably need another abstraction encapsulating a disjointed Graph of board states. With each component of the graph forming a Tree of a fixed depth equal to the number of turns you wish to take into account, and with the root describing a possible next move.

By the way, using 64 element byte array to describe the board will result in more efficient memory utilization (single array vs 9 arrays + pieces) and smaller memory footprint when it comes to generating a Graph of states.

Code style

Methods with high Cognitive Load

As I said before, I advise against including this method into the ChessBoardState API. The following suggestions only pertain to the coding style and can be applied to any similar scenario.

public List<ChessBoardState> expand(final PlayerTurn playerTurn) {
    
    final List<ChessBoardState> children = new ArrayList<>();
    
    if (null == playerTurn) {
        throw new NullPointerException("playerTurn is null.");
    } else switch (playerTurn) {
        case WHITE -> {
            for (int rank = 0; rank < N; rank++) {
                for (int file = 0; file < N; file++) {
                    final CellType cellType = getCellType(file, rank);
                    
                    if (cellType == CellType.WHITE) {
                        children.addAll(
                            state[rank]
                                [file].expand(this, file, rank));
                    }
                }
            }
        }
        
        case BLACK -> throw new UnsupportedOperationException();
        
        default -> throw new EnumConstantNotPresentException(
            PlayerTurn.class,
            playerTurn.name());
    }
    
    return children;
}

Issues:

  • Objects.requireNonNull() can be used to validate elements. But when it comes to internal APIs, it doesn't seem justifiable to validate each and every reference-type parameter. It better to work on eradicating practices of passing null values around.
  • else is not need when we're throwing from the if, it only adds another level of nesting.
  • Avoid this pattern: case SOME_LABEL -> { lot's of code } as it reduces code readability and diminishes the benefits of using switch expressions. Instead, extract the logic within the curly braces into a separate method with a descriptive name that communicates the purpose of the code block. Always refer to have a concise and simple expression associated with each case.

Complex initialization logic in Constructors

No-arguments constructor and copy-constructor of ChessBoardState are teaming with code.

It's better to keep constructors dumb and short. When a complex initialization required, consider defining a method that does the job of preparing the state and passes the state to a constructor.

So, no-args constructor new Board() producing a chess board with all pieces at their initial positions can be replaced with a static method Board.withInitialSetup().

And either an instance or a static method can take care of the copy-constructor's responsibility: new Board(original) becomes original.copy().

Benefits: names communicating intents, no heavy logic inside constructors.

\$\endgroup\$
7
  • \$\begingroup\$ "What's the difference between two instances of Piece with the same type and color? There's none." In fact, there is, although quite a subtle one: if such pieces change places, the resulting board is different from the original, and a three-fold repetition rule does not apply. In case I've misread you, my apologies. \$\endgroup\$ Commented Jul 2, 2024 at 22:10
  • \$\begingroup\$ @vnp There's no difference between instances of Piece of the same color and type because type, color and expander are their only attributes (see how pieces instantiated in the question and Piece definition in the OP's repository). \$\endgroup\$ Commented Jul 11, 2024 at 15:55
  • 1
    \$\begingroup\$ @vnp OP's pieces are not aware of their positions on the board. If you're advising to change this, then I already shared my opinion on such a design decision in the answer. The board should be responsible for describing positions of the pieces, and the board can track the history of moves. And a separate component should be tasked with analyzing the board state (like determining whether it's a check, checkmate, or draw), not a board. \$\endgroup\$ Commented Jul 11, 2024 at 15:56
  • 1
    \$\begingroup\$ @SimonForsberg Also, method Piece.move() will be triggering changes in the state of the Boar and its own state (I assume that a piece that knows how it can move is stateful) and potentially the state of another piece. Imaging how castling might look like is such a scenario... Making only one component responsible for moving pieces will make the implementation cleaner and simpler. \$\endgroup\$ Commented Sep 20, 2024 at 16:39
  • 1
    \$\begingroup\$ @SimonForsberg And taking into account that it's not the most juicy part of this project (since OP's ultimate goal is to create a chess engine), it's even more important to reduce the coupling and simplify these bare-bones elements, in order to make debugging and testing easier. \$\endgroup\$ Commented Sep 20, 2024 at 16:39
4
\$\begingroup\$

Bug

You have four fields:

private Piece[][] state;
private boolean[] whiteIsPreviouslyDoubleMoved = new boolean[N];
private boolean[] blackIsPreviouslyDoubleMoved = new boolean[N];
private byte enPassantFlags;

How many of these do you use in equals() and hashCode() and how many do you clear in clear() ?

If the pieces are in the same locations but the double-moved flags look different, are they boards really the same? No, they shouldn't be!

Make sure to use all fields in equals, hashCode and clear

It could possibly also be useful to have it in toString but that's slightly less important.

Defensive copies

Another possible future bug is that getWhiteIsPreviouslyDoubleMoved and getBlackIsPreviouslyDoubleMoved returns the underlying array, which is mutable. I know, I know, you have noted them that they are only used in unit tests. But that might not always be the case for all the future.

Be kind to your future self and make a defensive copy of the arrays to return instead, guaranteeing that the underlying state will never be mutated by someone else.

\$\endgroup\$
1
  • \$\begingroup\$ Correct implementation of equals() and hashCode() is of high-importance for the chess board implementation. Since there are several cases when two board states should be compared. Good catch. \$\endgroup\$ Commented Sep 20, 2024 at 16:51

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.