4
\$\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\$

2 Answers 2

6
\$\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\$
6
\$\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 and Pieces

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 and a set of non-zero values designated to describe pieces.

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

By describing a chess piece with a class or a record, we cannot leverage object-orientation's most powerful weapon: defining behavior. Because implementing move(), capture(), castle(), etc. requires knowing the locations of other pieces (i.e. we need to encroach on the area of responsibility of the board).

And the idea of a smart chess piece having a knowledge of the expander is the tail wagging the dog. If a Piece 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. What's the difference between two instances of Piece with the same type and color? There's none, then why spawning new instances? Furthermore, Board needs to know how to instantiate Piece and if Piece is coupled with the expander, then we failed to decouple the board and the expander implementations.

As a possible option an enum can be used to describe all chess pieces (WHITE_QUEEN, WHITE_KNIGHT, etc.), but in the absence of attributes it 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\$
3
  • \$\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\$
    – vnp
    Commented Jul 2 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 at 15:55
  • \$\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 at 15:56

Not the answer you're looking for? Browse other questions tagged or ask your own question.