Is there a better way to handle move validation in my chess program?

Question

I feel like my project could use some serious restructuring, but I am not sure where to start. I'm not sure if I am making it all more complex than it needs to be by doing a lot of different things in one place, or whether it is just inherently complex.

I am mostly concerned about the check for whether a move puts the player's own king in check. As of right now, I am performing the move and then checking if the king is in check, if so I undo the move by reversing the logic of the move type, and throwing an error to Game to be handled there. I really don't think this is the best way to do it in terms of simplicity and maintainability. I would appreciate any input, thanks!

public Move movePiece(Position position, Position destination, boolean isWhiteTurn) throws IllegalMoveException {
        ChessPiece thisPiece = confirmPiece(position, isWhiteTurn);  // Confirms the piece exists and is correct team
        validateMove(thisPiece, position, destination);  // Confirms the validity of the move for the piece
        ChessPiece otherPiece = getPiece(destination);
        Move.MoveType moveType = determineMoveType(thisPiece, position, otherPiece, destination);
        Move.CheckType checkType = determineCheckType(thisPiece, otherPiece);
        
        try {
            performMove(thisPiece, position, otherPiece, destination, moveType);  // Handles own king in check
        } catch (IllegalMoveException e) {
            undoMove(thisPiece, position, otherPiece, destination, moveType);
            throw new IllegalMoveException("Move puts own king in check");
        }

    ...
}

private void performMove(ChessPiece thisPiece, Position position, ChessPiece otherPiece,
                             Position destination, Move.MoveType moveType) throws IllegalMoveException {
        if (moveType == Move.MoveType.CASTLE) {
            performCastle(thisPiece, position, otherPiece, destination);
        }
        else if (moveType == Move.MoveType.EN_PASSANT) {
            performEnPassant(thisPiece, position, otherPiece, destination);
        }
        else if (otherPiece != null) {
            capturePiece(otherPiece);
            movePiece(thisPiece, position, destination);
        }
        else {
            movePiece(thisPiece, position, destination);
        }
        
        Position kingPos = findKingPos(thisPiece.isWhite());
        
        if (isAttacked(!thisPiece.isWhite(), kingPos)) {
            throw new IllegalMoveException("Move puts own king in check");
        }
    }

    private void undoMove(ChessPiece thisPiece, Position position, ChessPiece otherPiece, Position destination, Move.MoveType moveType) {
        if (moveType == Move.MoveType.CASTLE) {
            undoCastle(thisPiece, position, otherPiece, destination);
        }
        if (moveType == Move.MoveType.EN_PASSANT) {
            undoEnPassant(thisPiece, position, otherPiece, destination);
        }
        else {
            undoMove(thisPiece, position, destination);
            if (otherPiece != null) {
                undoCapture(otherPiece, destination);
            }
        }
    }
    
    private void capturePiece(ChessPiece piece) {
        Position position = piece.getPosition();
        removePiece(position);
        if (piece.isWhite()) {
            whitePieces.remove(position);
            blackCaptured.add(piece);
        } else {
            blackPieces.remove(position);
            whiteCaptured.add(piece);
        }
    }
    
    private void movePiece(ChessPiece piece, Position position, Position destination) {
        if (getPiece(destination) != null) {
            throw new IllegalArgumentException("There is a piece there");
        }
        removePiece(position);
        setPiece(piece, destination);
        if (piece.isWhite()) {
            whitePieces.remove(position);
            whitePieces.put(destination, piece);
        } else {
            blackPieces.remove(position);
            blackPieces.put(destination, piece);
        }
    }
    
    private void undoMove(ChessPiece piece, Position position, Position destination) {
        movePiece(piece, destination, position);
    }
    
    private void undoCapture(ChessPiece piece, Position position) {
        setPiece(piece, position);
        if (piece.isWhite()) {
            whitePieces.put(position, piece);
            blackCaptured.remove(piece);
        } else {
            blackPieces.put(position, piece);
            whiteCaptured.remove(piece);
        }
    }

    
    private void performCastle(ChessPiece thisPiece, Position position, ChessPiece otherPiece, Position destination) {
        if (thisPiece instanceof King) {
            if (destination.col() == 0) {
                performQueenSideCastle(thisPiece, position, otherPiece, destination);
            } else {
                performKingSideCastle(thisPiece, position, otherPiece, destination);
            }
        } else {
            if (destination.col() == 0) {
                performQueenSideCastle(otherPiece, destination, thisPiece, position);
            } else {
                performKingSideCastle(otherPiece, destination, thisPiece, position);
            }
        }

    }
    
    private void undoCastle(ChessPiece thisPiece, Position position, ChessPiece otherPiece, Position destination) {
        if (thisPiece instanceof King) {
            if (destination.col() == 0) {
                undoQueenSideCastle(thisPiece, position, otherPiece, destination);
            } else {
                undoKingSideCastle(thisPiece, position, otherPiece, destination);
            }
        } else {
            if (destination.col() == 0) {
                undoQueenSideCastle(otherPiece, destination, thisPiece, position);
            } else {
                undoKingSideCastle(otherPiece, destination, thisPiece, position);
            }
        }
    }
    
    private void performQueenSideCastle(ChessPiece king, Position kingPosition, ChessPiece rook, Position rookPosition) {
        if (king.isWhite()) {
            movePiece(king, kingPosition, new Position(0, 2));
            movePiece(rook, rookPosition, new Position(0, 3));
        } else {
            movePiece(king, kingPosition, new Position(7, 2));
            movePiece(rook, rookPosition, new Position(7, 3));
        }
    }
    
    private void undoQueenSideCastle(ChessPiece king, Position kingPosition, ChessPiece rook, Position rookPosition) {
        if (king.isWhite()) {
            undoMove(king, kingPosition, new Position(0, 2));
            undoMove(rook, rookPosition, new Position(0, 3));
        } else {
            undoMove(king, kingPosition, new Position(7, 2));
            undoMove(rook, rookPosition, new Position(7, 3));
        }
    }
    
    
    private void performKingSideCastle(ChessPiece king, Position kingPosition, ChessPiece rook, Position rookPosition) {
        if (king.isWhite()) {
            movePiece(king, kingPosition, new Position(0, 6));
            movePiece(rook, rookPosition, new Position(0, 5));
        } else {
            movePiece(king, kingPosition, new Position(7, 6));
            movePiece(rook, rookPosition, new Position(7, 5));
        }
    }
    
    private void undoKingSideCastle(ChessPiece king, Position kingPosition, ChessPiece rook, Position rookPosition) {
        if (king.isWhite()) {
            undoMove(king, kingPosition, new Position(0, 6));
            undoMove(rook, rookPosition, new Position(0, 5));
        } else {
            undoMove(king, kingPosition, new Position(7, 6));
            undoMove(rook, rookPosition, new Position(7, 5));
        }
    }
    
    private void performEnPassant(ChessPiece thisPiece, Position position, ChessPiece otherPiece, Position destination) {
        movePiece(thisPiece, position, destination);
        capturePiece(otherPiece);
    }
    
    private void undoEnPassant(ChessPiece thisPiece, Position position, ChessPiece otherPiece, Position destination) {
        undoMove(thisPiece, position, destination);
        undoCapture(otherPiece, otherPiece.getPosition());
    }
    

public record Move(Position position, Position destination, ChessPiece movedPiece, ChessPiece capturedPiece, Move.MoveType moveType, Move.CheckType checkType) {
    public enum MoveType {
        NORMAL, CASTLE, EN_PASSANT, PROMOTION
    }
    
    public enum CheckType {
        NONE, CHECK, STALEMATE, CHECKMATE
    }
}

└── src
    ├── controller
    │   └── Game.java
    ├── model
    │   ├── Bishop.java
    │   ├── Board.java
    │   ├── ChessPiece.java
    │   ├── King.java
    │   ├── Knight.java
    │   ├── Pawn.java
    │   ├── Queen.java
    │   └── Rook.java
    └── util
        ├── ChessPieceFactory.java
        ├── IllegalMoveException.java
        ├── Move.java
        └── Position.java

Answer 1

One major issue is that your exception is called IllegalMoveException, but the catching code in your top-level assumes it's an invalid move due to the king being moved. You should not do that - if it is an invalid move, undo the move, and let that exception bubble up. Don't toss that exception in the bin and make a new one with the text 'it must have been putting your own king in check'.

The principle of 'move, then check, if fail, unmove' is not necessarily a bad idea. If you really want to go far on this, go for an immutable data model. You don't move pieces on the board - you instead construct a new board. This is how strings work:

String x = "Hello!";
x.toLowerCase();
System.out.println(x); // prints Hello!, with uppercase

That's because strings are immutable. toLowerCase() doesn't change the string you call it on, it produces a new one. You'd have to write x = x.toLowerCase(). You could set things up the same way. It's 'more pricey', in that you're copying a ton of boards this way, but a chess board is not all that large, and it lets you do interesting things, such as using board objects as keys in a map, so that you can memoize any point value calculations you do. Note that 'whose turn is it' is part of a board.

That way, you can generate the new board as 'proposed' by the move, check this new board for validity. If it isn't valid, just toss it and move on. If it is valid, replace 'this is the current board' with the newly generated board and go from there. This also means you can track the game (it's a sequence of board states, after all).