8
\$\begingroup\$

(This post has now a continuation.)

I decided to embark on implementing my own chess engine. The first (and perhaps most demanding) part of that endeavour is generating child states out of a given chess board state. Below, you can see my attempt to generate (only) movements of white pawns. The code also mentions a little bit of logic for moving black pawns, yet I don't want that part reviewed since it will eventually become sort of symmetric to moving the white pawns. Finally, the entire GitHub repo is here.

Code

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

package com.github.coderodde.game.chess;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

/**
 * This class implements a chess board state.
 * 
 * @version 1.0.0 (Jun 22, 2024)
 * @since 1.0.0 (Jun 22, 2024)
 */
public final class ChessBoardState {
    
    private static final int N = 8;
    
    public static final int EMPTY        = 0;
    public static final int WHITE_PAWN   = 1;
    public static final int WHITE_BISHOP = 2;
    public static final int WHITE_KNIGHT = 3;
    public static final int WHITE_ROOK   = 4;
    public static final int WHITE_QUEEN  = 5;
    public static final int WHITE_KING   = 6;
 
    public static final int BLACK_PAWN   = 9;
    public static final int BLACK_BISHOP = 10;
    public static final int BLACK_KNIGHT = 11;
    public static final int BLACK_ROOK   = 12;
    public static final int BLACK_QUEEN  = 13;
    public static final int BLACK_KING   = 14;
    
    private static final int CELL_COLOR_NONE  = 0;
    private static final int CELL_COLOR_WHITE = +1;
    private static final int CELL_COLOR_BLACK = -1;
    
    private int[][] state = new int[N][N];
    private boolean[] whiteIsPreviouslyDoubleMoved = new boolean[N];
    private boolean[] blackIsPreviouslyDoubleMoved = new boolean[N];
    
    public ChessBoardState() {
        
        // Black pieces:
        state[0][0] =
        state[0][7] = BLACK_ROOK;
  
        state[0][1] = 
        state[0][6] = BLACK_KNIGHT;
        
        state[0][2] = 
        state[0][5] = BLACK_BISHOP;
  
        state[0][3] = BLACK_QUEEN;
        state[0][4] = BLACK_KING;
        
        for (int x = 0; x < N; x++) {
            state[1][x] = BLACK_PAWN;
        }
        
        // White pieces:
        state[7][0] =
        state[7][7] = WHITE_ROOK;
  
        state[7][1] = 
        state[7][6] = WHITE_KNIGHT;
        
        state[7][2] = 
        state[7][5] = WHITE_BISHOP;
        
        state[7][3] = WHITE_QUEEN;
        state[7][4] = WHITE_KING;
        
        for (int x = 0; x < N; x++) {
            state[6][x] = WHITE_PAWN;
        }
    }
    
    public ChessBoardState(final ChessBoardState copy) {
        this.state = new int[N][N];
        
        for (int y = 0; y < N; y++) {
            System.arraycopy(copy.state[y], 0, this.state[y], 0, N);
        }
        
        this.whiteIsPreviouslyDoubleMoved = copy.whiteIsPreviouslyDoubleMoved;
        this.blackIsPreviouslyDoubleMoved = copy.blackIsPreviouslyDoubleMoved;
    }
    
    @Override
    public boolean equals(final Object o) {
        if (o == this) {
            return true;
        }
        
        if (o == null) {
            return false;
        }
        
        if (!getClass().equals(o.getClass())) {
            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 int[N][N];
    }
    
    /**
     * Returns the piece value at rank {@code y}, file {@code x}. Used in unit 
     * testing.
     * 
     * @param x the file of the requested piece.
     * @param y the rank of the requested piece.
     * 
     * @return the piece value.
     */
    public int get(final int x, final int y) {
        return state[y][x];
    }
    
    /**
     * Sets the piece {@code piece} at rank {@code y}, file {@code x}. Used in 
     * unit testing.
     * 
     * @param x the file of the requested piece.
     * @param y the rank of the requested piece.
     * 
     * @param piece the value of the desired piece.
     */
    public void set(final int x, final int y, final int piece) {
        state[y][x] = piece;
    }
    
    /**
     * Clears the position at rank {@code y} and file {@code x}. Used in unit 
     * testing.
     * 
     * @param x the file of the requested piece.
     * @param y the rank of the requested piece.
     */
    public void clear(final int x, final int y) {
        state[y][x] = EMPTY;
    }
    
    /**
     * Returns a simple textual representation of this state. Not very readable.
     * 
     * @return a textual representation of this state.
     */
    @Override
    public String toString() {
        final StringBuilder stringBuilder =
                new StringBuilder((N + 3) * (N + 2));
        
        int rankNumber = 8;
        
        for (int y = 0; y < N; y++) {
            for (int x = -1; x < N; x++) {
                if (x == -1) {
                    stringBuilder.append(rankNumber--).append(' ');
                } else {
                    stringBuilder.append(
                            convertPieceCodeToUnicodeCharacter(x, y));
                }
            }
            
            stringBuilder.append('\n');
        }
        
        stringBuilder.append("\n  abcdefgh");
        return stringBuilder.toString();
    }
    
    public List<ChessBoardState> expand(final PlayerTurn playerTurn) {
        
        final List<ChessBoardState> children = new ArrayList<>();
        
        if (playerTurn == PlayerTurn.WHITE) {
            for (int y = 0; y < N; y++) {
                for (int x = 0; x < N; x++) {
                    final int cellColor = getCellColor(x, y);

                    if (cellColor != CELL_COLOR_WHITE) {
                        continue;
                    }

                    expandWhiteMovesImpl(children, x, y);
                }
            }
        } else { // playerTurn == PlayerTurn.BLACK
            for (int y = 0; y < N; y++) {
                for (int x = 0; x < N; x++) {
                    final int cellColor = getCellColor(x, y);

                    if (cellColor != CELL_COLOR_BLACK) {
                        continue;
                    }

                    expandBlackMovesImpl(children, x, y);
                }
            }    
        }
        
        return children;
    }
    
    /**
     * Marks that the white pawn at file {@code x} made an initial double move.
     * Used for unit testing.
     * 
     * @param x the file number of the target white pawn. 
     */
    public void markWhitePawnInitialDoubleMove(final int x) {
        this.whiteIsPreviouslyDoubleMoved[x] = true;
    }
    
    /**
     * Marks that the black pawn at file {@code x} made an initial double move.
     * Used for unit testing.
     * 
     * @param x the file number of the target white pawn. 
     */
    public void markBlackPawnInitialDoubleMove(final int x) {
        this.blackIsPreviouslyDoubleMoved[x] = true;
    }
    
    private void expandWhiteMovesImpl(final List<ChessBoardState> children,
                                      final int x,
                                      final int y) {
        
        unmarkAllInitialWhiteDoubleMoveFlags();
        
        final int cell = state[y][x];
        
        switch (cell) {
            case WHITE_PAWN:
                expandImplWhitePawn(children, x, y);
                break;
                
            case WHITE_ROOK:
                break;
                
            case WHITE_BISHOP:
                break;
                
            case WHITE_KNIGHT:
                break;
                
            case WHITE_QUEEN:
                break;
                
            case WHITE_KING:
                break;
                
            default:
                throw new IllegalStateException("Should not get here.");
        }
    }
    
    private void expandBlackMovesImpl(final List<ChessBoardState> children,
                                      final int x,
                                      final int y) {
        throw new UnsupportedOperationException();
    }
    
    private void expandImplWhitePawn(final List<ChessBoardState> children,
                                     final int x,
                                     final int y) {
        
        if (y == 6 && state[5][x] == EMPTY 
                   && state[4][x] == EMPTY) {
           
            // Once here, can move the white pawn two moves forward:
            final ChessBoardState child = new ChessBoardState(this);

            child.markWhitePawnInitialDoubleMove(x);
            
            child.state[6][x] = EMPTY; 
            child.state[4][x] = WHITE_PAWN;
            
            children.add(child);
            
            this.markWhitePawnInitialDoubleMove(x);
        }
        
        if (y == 3) {
            // Try en passant, white pawn can capture a black onen?
            if (x > 0) {
                // Try en passant to the left:
                enPassantWhitePawnToLeft(x, children);
            }
            
            if (x < N - 1) {
                // Try en passant to the right:
                enPassantWhitePawnToRight(x, children);
            }
        }
        
        if (state[y - 1][x] == EMPTY && y == 1) {
            // Once here, can do promotion:
            addWhitePromotion(children,
                              this,
                              x);
            return;
        }
        
        // Move forward:
        if (y > 0 && getCellColor(x, y - 1) == CELL_COLOR_NONE) {
            // Once here, can move forward:
            ChessBoardState child = new ChessBoardState(this);

            child.state[y][x] = EMPTY;
            child.state[y - 1][x] = WHITE_PAWN;
            children.add(child);
        }
        
        if (x > 0 && y > 0 && getCellColor(x - 1, y - 1) == CELL_COLOR_BLACK) {
            // Once here, can capture to the left:
            final ChessBoardState child = new ChessBoardState(this);
            
            child.state[y][x] = EMPTY;
            child.state[y - 1][x - 1] = WHITE_PAWN;
            
            children.add(child);
        }
        
        if (x < N - 1 && y > 0 
                      && getCellColor(x + 1, y - 1) == CELL_COLOR_BLACK) {
            // Once here, can capture to the right:
            final ChessBoardState child = new ChessBoardState(this);
            
            child.state[y][x] = EMPTY;
            child.state[y - 1][x + 1] = WHITE_PAWN;
            
            children.add(child);
        }
    }
    
    /**
     * Tries to perform an en passant by the white pawn at the file {@code x} 
     * to a black pawn at the file {@code x - 1}.
     * 
     * @param x        the file of the capturing white pawn.
     * @param children the list of child n
     */
    private void enPassantWhitePawnToLeft(
            final int x, 
            final List<ChessBoardState> children) {
        
        if (!blackIsPreviouslyDoubleMoved[x - 1]) {
            return;
        }
        
        final ChessBoardState child = new ChessBoardState(this);
        
        child.clear(x, 3);
        child.clear(x - 1, 3);
        child.set(x - 1, 2, WHITE_PAWN);
        
        children.add(child);
    }
    
    private void enPassantWhitePawnToRight(
            final int x,
            final List<ChessBoardState> children) {
        
        if (!blackIsPreviouslyDoubleMoved[x + 1]) {
            return;
        }
        
        final ChessBoardState child = new ChessBoardState(this);
        
        child.clear(x, 3);
        child.clear(x + 1, 3);
        child.set(x + 1, 2, WHITE_PAWN);
        
        children.add(child);
    }
    
    private void unmarkAllInitialWhiteDoubleMoveFlags() {
        for (int i = 0; i < N; i++) {
            this.whiteIsPreviouslyDoubleMoved[i] = false;
        }
    }
    
    private void expandImplBlackPawn(final List<ChessBoardState> children,
                                     final int x,
                                     final int y) {
        
        if (y == 6 && state[2][x] == EMPTY 
                   && state[3][x] == EMPTY) {
            
            // Once here, can move the black pawn two moves forward:
            final ChessBoardState child = new ChessBoardState(this);
//            
//            child.unsetBlackInitialMovePawn(x);
            child.state[2][x] = EMPTY;
            child.state[4][x] = BLACK_PAWN;
        }
        
    }
    
    private void addWhitePromotion(
            final List<ChessBoardState> children,
            final ChessBoardState state,
            final int x) {
        
        ChessBoardState child = new ChessBoardState(state);
        child.state[0][x] = WHITE_QUEEN;
        child.state[1][x] = EMPTY;
        children.add(child);
        
        if (x > 0 && getCellColor(x - 1, 0) == CELL_COLOR_BLACK) {
            // Can capture/promote to the left:
            child = new ChessBoardState(state);
            child.state[0][x - 1] = WHITE_QUEEN;
            child.state[1][x] = EMPTY;
            children.add(child);
        }
        
        if (x < N - 1 && getCellColor(x + 1, 0) == CELL_COLOR_BLACK) {
            // Can capture/promote to the right:
            child = new ChessBoardState(state);
            child.state[0][x + 1] = WHITE_QUEEN;
            child.state[1][x] = EMPTY;
            children.add(child);
        }
    }
    
    private void addBlackPromotion(final List<ChessBoardState> children,
                                   final ChessBoardState state,
                                   final int x) {
        
        final ChessBoardState child = new ChessBoardState(state);
        child.state[7][x] = BLACK_QUEEN;
        children.add(child);
    }
    
    /**
     * Returns the color of the cell at file {@code (x + 1)} and rank 
     * {@code 8 - y}.
     * 
     * @param x the file index.
     * @param y the rank index.
     * 
     * @return {@link #CELL_COLOR_NONE} if the requested cell is empty,
     *         {@link #CELL_COLOR_WHITE} if the requested cell is white, and,
     *         {@link #CELL_COLOR_BLACK} if the requested cell is black.
     */
    private int getCellColor(final int x, final int y) {
        final int cell = state[y][x];
        
        if (cell == EMPTY) {
            return CELL_COLOR_NONE;
        }
        
        return 0 < cell && cell < 7 ? CELL_COLOR_WHITE : 
                                      CELL_COLOR_BLACK;
    }
    
    private char convertPieceCodeToUnicodeCharacter(final int x, final int y) {
        final int pieceCode = state[y][x];
        
        switch (pieceCode) {
            case EMPTY -> {
                return (x + y) % 2 == 0 ? '.' : '#';
            }
                
            case WHITE_PAWN -> {
                return 'P';
            }
                
            case WHITE_KNIGHT -> {
                return 'K';
            }
                
            case WHITE_BISHOP -> {
                return 'B';
            }
                
            case WHITE_ROOK -> {
                return 'R';
            }
                
            case WHITE_QUEEN -> {
                return 'Q';
            }
                
            case WHITE_KING -> {
                return 'X';
            }
                
            case BLACK_PAWN -> {
                return 'p';
            }
                
            case BLACK_KNIGHT -> {
                return 'k';
            }
                
            case BLACK_BISHOP -> {
                return 'b';
            }
                
            case BLACK_ROOK -> {
                return 'r';
            }
                
            case BLACK_QUEEN -> {
                return 'q';
            }
                
            case BLACK_KING -> {
                return 'x';
            }
                
            default -> throw new IllegalStateException("Should not get here.");
        }
    }
}

The unit test class follows.

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

package com.github.coderodde.game.chess;

import static com.github.coderodde.game.chess.ChessBoardState.BLACK_BISHOP;
import static com.github.coderodde.game.chess.ChessBoardState.BLACK_KNIGHT;
import static com.github.coderodde.game.chess.ChessBoardState.BLACK_PAWN;
import static com.github.coderodde.game.chess.ChessBoardState.BLACK_ROOK;
import static com.github.coderodde.game.chess.ChessBoardState.WHITE_BISHOP;
import static com.github.coderodde.game.chess.ChessBoardState.WHITE_PAWN;
import static com.github.coderodde.game.chess.ChessBoardState.WHITE_QUEEN;
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 ChessBoardStateTest {
    
    private ChessBoardState state;
    
    @Before
    public void before() {
        state = new ChessBoardState();
        state.clear();
    }
    
    @Test
    public void moveWhitePawnInitialDoubleMove() {
        state.set(0, 6, WHITE_PAWN);
        
        final List<ChessBoardState> children = state.expand(PlayerTurn.WHITE);
        
        assertEquals(2, children.size());
        
        final ChessBoardState child1 = children.get(0);
        final ChessBoardState child2 = children.get(1);
        
        assertTrue(children.contains(child1));
        assertTrue(children.contains(child2));
        
        final Set<Integer> indexSet = new HashSet<>();
        
        indexSet.add(children.indexOf(child1));
        indexSet.add(children.indexOf(child2));
        
        assertEquals(2, indexSet.size());
        
        final ChessBoardState move1 = new ChessBoardState();
        final ChessBoardState move2 = new ChessBoardState();
        
        move1.clear();
        move2.clear();
        
        move1.set(0, 5, WHITE_PAWN);
        move2.set(0, 4, WHITE_PAWN);
        
        assertTrue(children.contains(move1));
        assertTrue(children.contains(move2));
    }
    
    @Test
    public void whitePawnCannotMoveForward() {
        state.set(4, 5, WHITE_PAWN);
        state.set(4, 4, BLACK_ROOK);
        
        final List<ChessBoardState> children = state.expand(PlayerTurn.WHITE);
        
        assertTrue(children.isEmpty());
    }
    
    @Test
    public void whitePawnCanEatBothDirectionsAndMoveForward() {
        state.set(5, 4, WHITE_PAWN);
        state.set(4, 3, BLACK_KNIGHT);
        state.set(6, 3, BLACK_ROOK);
        
        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();
        
        // Eat to the left:
        move1.set(4, 3, WHITE_PAWN);
        move1.set(6, 3, BLACK_ROOK);
        
        // Move forward:
        move2.set(5, 3, WHITE_PAWN);
        move2.set(4, 3, BLACK_KNIGHT);
        move2.set(6, 3, BLACK_ROOK);
        
        // Eat to the right:
        move3.set(6, 3, WHITE_PAWN);
        move3.set(4, 3, BLACK_KNIGHT);
        
        assertTrue(children.contains(move1));
        assertTrue(children.contains(move2));
        assertTrue(children.contains(move3));
    }
    
    @Test
    public void whitePawnCannotMakeFirstDoubleMoveDueToObstruction() {
        state.set(6, 6, WHITE_PAWN);
        state.set(6, 5, WHITE_BISHOP);
        
        assertTrue(state.expand(PlayerTurn.WHITE).isEmpty());
        
        state.clear();
        state.set(4, 6, WHITE_PAWN);
        state.set(4, 5, BLACK_ROOK);
        
        assertTrue(state.expand(PlayerTurn.WHITE).isEmpty());
    }
    
    @Test
    public void whitePawnPromotion() {
        state.set(3, 1, WHITE_PAWN);
        final List<ChessBoardState> children = state.expand(PlayerTurn.WHITE);
        
        assertEquals(1, children.size());
        
        final ChessBoardState move = new ChessBoardState();
        
        move.clear();
        move.set(3, 0, WHITE_QUEEN);
        
        assertEquals(move, children.get(0));
    }
    
    @Test
    public void whitePawnPromotionCaptureBoth() {
        state.set(5, 1, WHITE_PAWN);
        state.set(4, 0, BLACK_BISHOP);
        state.set(6, 0, BLACK_PAWN);
        
        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();
        
        // Queen forward:
        move1.set(4, 0, BLACK_BISHOP);
        move1.set(5, 0, WHITE_QUEEN);
        move1.set(6, 0, BLACK_PAWN);
        
        // Queen left:
        move2.set(4, 0, WHITE_QUEEN);
        move2.set(6, 0, BLACK_PAWN);
        
        // Queen right:
        move3.set(6, 0, WHITE_QUEEN);
        move3.set(4, 0, BLACK_BISHOP);
        
        assertTrue(children.contains(move1));
        assertTrue(children.contains(move2));
        assertTrue(children.contains(move3));
        
        final Set<Integer> indexSet = new HashSet<>();
        
        indexSet.add(children.indexOf(move1));
        indexSet.add(children.indexOf(move2));
        indexSet.add(children.indexOf(move3));
        
        assertEquals(3, indexSet.size());
    }
    
    @Test
    public void whitePawnEnPassantToLeft() {
        state.set(0, 3, BLACK_PAWN);
        state.set(1, 2, BLACK_ROOK);
        state.set(1, 3, WHITE_PAWN);
        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, 2, BLACK_ROOK);
        move.set(0, 2, WHITE_PAWN);
        
        assertTrue(children.contains(move));
    }
    
    @Test
    public void whitePawnEnPassantToRight() {
        state.set(7, 3, BLACK_PAWN);
        state.set(6, 2, BLACK_ROOK);
        state.set(6, 3, WHITE_PAWN);
        state.markBlackPawnInitialDoubleMove(7);
        
        final List<ChessBoardState> children = state.expand(PlayerTurn.WHITE);
        
        assertEquals(1, children.size());
        
        final ChessBoardState move = new ChessBoardState();
        
        move.clear();
        move.set(6, 2, BLACK_ROOK);
        move.set(7, 2, WHITE_PAWN);
        
        assertTrue(children.contains(move));
    }
}

Critique request

First of all, do I break the rules? Also, I would like whatever comes to mind.

\$\endgroup\$
1
  • \$\begingroup\$ Note a pawn may promote to any type of piece other than pawn or king, not just queen. Of course, queen is usually the most useful. (There are classic chess puzzles where choosing queen causes a draw but choosing a knight leads to a win.) \$\endgroup\$
    – aschepler
    Commented 2 days ago

4 Answers 4

11
\$\begingroup\$

The code in general is fine. If it were a C program (or some other classic language) I would even say good. However this is Java and especially by using integers to represent the pieces you completely miss its point, which includes type safety and would eliminate things like throw new IllegalStateException("Should not get here.").

This seems like a prime example to use Enums and records:

enum PieceType {
    PAWN, BISHOP, KNIGHT, ROOK, QUEEN, KING;
};

enum Color {
    WHITE, BLACK;
}

record Piece(PieceType type, Color color) {}

private Piece[][] state = new Piece[N][N];

state[0][0] =
state[0][7] = new Piece(ROOK, WHITE);

Empty squares would be represented by null.

This could be extended further by putting piece type specific code such as the character representation and move validation into the PieceType instances.


If you do want to use integers instead, then at the least consider using bit manipulation, for example:

public static final byte EMPTY = 0;

public static final byte PAWN   = (byte) 0b0000001;
public static final byte BISHOP = (byte) 0b0000010;
public static final byte KNIGHT = (byte) 0b0000100;
public static final byte ROOK   = (byte) 0b0001000;
public static final byte QUEEN  = (byte) 0b0010000;
public static final byte KING   = (byte) 0b0100000;

public static final byte WHITE_PAWN   = PAWN;
public static final byte WHITE_BISHOP = BISHOP;
public static final byte WHITE_KNIGHT = KNIGHT;
public static final byte WHITE_ROOK   = ROOK;
public static final byte WHITE_QUEEN  = QUEEN;
public static final byte WHITE_KING   = KING;

public static final byte BLACK = (byte) 0b1000000;

public static final byte BLACK_PAWN   = BLACK | PAWN;
public static final byte BLACK_BISHOP = BLACK | BISHOP;
// etc.

This allows simple checking of type:

if (piece & PAWN > 0) { ... }

and color

if (piece & BLACK == 0) {
  // Piece is white
} else {
  // piece is black
}

Some other points:

if (!getClass().equals(o.getClass())) {

This would be simpler as

if (o instanceof ChessBoardState) {

This would also eliminate the null check before, because this avoids accessing a potential null reference, and null instanceof ... always returns false anyway.


You are misusing the switch expression syntax a bit. It could be written as:

return switch (pieceCode) {
  case EMPTY -> (x + y) % 2 == 0 ? '.' : '#';
              
  case WHITE_PAWN -> 'P';

  // etc.
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Modeling with null is certainly one design choice that could work. Another would be to introduce a third player who has only one kind of piece: EMPTY. Now we can’t NPE. \$\endgroup\$
    – J_H
    Commented Jun 26 at 15:40
  • 1
    \$\begingroup\$ There shouldn't be any danger from an NPE: 1) The developer should be aware of the use of null here 2) Any decent IDE nowadays will warn you. And instead of a "third player", a better solution would have an interface from which both the record Piece and an empty piece/field would inherit. \$\endgroup\$
    – RoToRa
    Commented Jun 26 at 18:36
10
\$\begingroup\$
  • The methods related to InitialDoubleMove are misnamed. It took me a while to understand what their purpose is. Consider renaming them to setEnPassant, clearEnPassant, etc.

    Speaking of en passant, I don't see the benefit of having an entire array of them. It may occur on at most a single square. In fact, it may occur at a single file (the rank must be 3). Something along the lines

      if (y == 3) {
          if (enPassantFile == x - 1 || enPassantFile == x + 1) {
              enPassantCapture(x, enPassantFile, children);
          }
      }
    

    Perhaps a single byte (with 8 bit flags, corresponding to 8 files) is sufficient.

  • Promotion is buggy. A test for a promotion with capture is done in addWhitePromotion - and it is too late: addWhitePromotion is called only if a regular move promotes. For example, in a position W: e7, B: Ke8, Rg8 the move egQ will not be generated.

    I recommend to check for the promotion after the move has been generated. Then the check is as simple as y == 1.

    BTW, you only promote to the Queen. It is not right for a realistic chess engine.

  • expandImplWhitePawn is very hard to follow. First, the test for y > 0 shall be done once, at the very beginning, and throw if fails. Second, I recommend restructuring it a bit differently:

      if (getCellColor(x, y - 1) == CELL_COLOR_NONE) {
          // generate a regular move forward
          if (y == 6 && getCellColor(x, y - 2) == CELL_COLOR_NONE) {
              // generate a double move forward
          }
      }
      // Now generate possible captures
      // Now check for promotion
    
  • It feels very uncomfortable that some parts of code test for getCellColor() == CELL_COLOR_NONE, and some directly test for state[][] == EMPTY.

  • Please no magic numbers. Use WHITE_INITIAL_RANK = 1 and WHITE_EN_PASSANT_RANK = 3.

    Also, just for a chess player's peace of mind use rank, file instead of x, y.

EDIT regarding a bug in the promotion logic.

Try a test case:

@Test
public void whitePawnPromotionCaptureBlocked() {
        state.set(5, 1, WHITE_PAWN);
        state.set(5, 0, BLACK_BISHOP);
        state.set(6, 0, BLACK_PAWN);

        // In this position there is one move,
        // which is Queen right
        // Unless I am blind, this move is not generated.
}
\$\endgroup\$
4
  • \$\begingroup\$ I am keeping missing the promotion is buggy part. Could you elaborate? \$\endgroup\$
    – coderodde
    Commented Jun 27 at 10:45
  • \$\begingroup\$ @coderodde Added a test case. \$\endgroup\$
    – vnp
    Commented Jun 27 at 21:42
  • 2
    \$\begingroup\$ @coderodde Whenever you feel inclined to write a comment to segregate a function into sections, these sections should probably be functions of their own. This can make your move-generation method Pawn.generateRegularMoves(...); Pawn.generateCapturingMoves(...); Pawn.generateEnpassantMoves(...); Pawn.handlePromotions(...) and each of these methods is very clear and short. \$\endgroup\$
    – Falco
    Commented Jun 28 at 9:28
  • 1
    \$\begingroup\$ @Falco I feel you. I will get back to your advice asap. \$\endgroup\$
    – coderodde
    Commented 2 days ago
6
\$\begingroup\$

Do you just want elegant code, or do you want efficiency?

I'm inclined to think that the memory allocated to a State needs to be a lot less than an array of 64 integers. It's fairly easy to get it down to 32 (max number of pieces) × 6 bits which is a factor of 10 smaller.

But looking at how the data is used also matters. Do you want to store the piece at each position, or the position of each piece? Perhaps you could do a bit of both: a 64-bit map indicating the squares occupied by white, another 64-bit map indicating the squares occupied by black, 16 bytes to hold the (x,y) position of each white piece, 16 bytes to hold the position of each black piece; total 48 bytes compared with your 256. It not only saves space, but copying the board involves moving less data, and of course smaller data structures lead to better CPU cache hits. And you still get pretty efficient access for the major operations: your 8×8 search to find all the white pieces is replaced by a scan of a 16-byte array, and the test of whether a square is occupied becomes a simple shift/mask test on a 64-bit integer.

Of course, the devil is in the detail. As well as en-passant and castling status, you need to allow for the board to hold extra Queens, etc, after a promotion.

Afterthought

Or perhaps:

  • 64-bit map showing squares occupied by white pieces/pawns
  • 64-bit map showing squares occupied by black pieces/pawns
  • 64-bit map showing squares occupied by pawns
  • variable-length array of 16-bit values for the major pieces - 3 bits to identify the piece (K|Q|B|R|N), 1 bit to identify the colour, 6 bits to identify the square, 6 bits spare

After writing this I've realised that I instinctively start an exercise like this by thinking about the data structures, and then build the code on top. An alternative approach is to design the interface to the State class first, and then design the data structure underneath.

\$\endgroup\$
1
  • \$\begingroup\$ This is my first attempt at chess AI and - thus - I am satisfied with not squeezing performance. \$\endgroup\$
    – coderodde
    Commented Jun 27 at 6:51
2
\$\begingroup\$

Very good first attempt.

Code follows C-style patterns.

For more Java/OOPS-style, try incorporating these into the code:

  1. Solid Patterns
    1.1. Segregate Actors and Actions
    1.2. Actors are Pieces on Board
    1.2.1. Have Data ex: position on board
    1.2.2. Have Actions for Data, ex: horse can move in 4 directions, pawn in one, etc
    1.3. Actions can be Strategy
    1.4. We can make class for engine with composition with Data Class + Actions
    1.5. Later Actions can come from AI Engine
    Bonus: Actions Validation Engine

  2. Use Lombok
    2.1. Code will be lot cleaner

  3. Create Interfaces
    3.1. Will make actions clearer

New contributor
Adi is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
\$\endgroup\$
2
  • 2
    \$\begingroup\$ Those are general advises applicable on any piece of software \$\endgroup\$ Commented Jun 27 at 9:24
  • 1
    \$\begingroup\$ @BillalBegueradj I agree. OP seems new to this so is this relevant? \$\endgroup\$
    – Adi
    Commented Jun 27 at 9:51

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