5
\$\begingroup\$

I was working on a checkers game for a college project.

After I did a sprint to finish it, I want to improve the movement logic.

Right now, it is housed within the Game.java class. I feel like the methods I've created are too large and convoluted and there must be a way to simplify this logic.

Additionally, I am concerned about overlapping logic in canJump() and canMove(), but I don't know how to separate them. My goal is to streamline these checks and make it easier to diagnose what the user did wrong.

This is a very simplified version of the game (coding it was already hard enough), where I do not have to handle king pieces or anything of the sort. The pieces can only move forwards and jump forwards when attacking. The rules themselves are the basic checkers rules.

Game.java:

package core;

/**
 * The game class. It is responsible for the logic and state of the checkers game.
 * @author Danylo Rybchynskyi
 */
public class Game {
    private final char emptySpot = '_';
    private final char[] players = {'X', 'O'};
    private final char[][] board = new char[8][8];
    private int currentPlayer;

    /**
     * Constructs the Game class and assigns the starting player index
     * @param startingPlayer the index of the starting player. 0 - X, 1 - O.
     */
    public Game(int startingPlayer) {
        // Set up the O pieces at the top of the board
        for (int i = 0; i < 3; ++i) {
            for (int j = 0; j < 8; ++j) {
                if ((i + j) % 2 == 1)
                    board[i][j] = players[1];
                else
                    board[i][j] = emptySpot;
            }
        }
        // Create blank spaces in the middle of the board
        for (int i = 3; i <= 4; ++i) {
            for (int j = 0; j < 8; ++j) {
                board[i][j] = emptySpot;
            }
        }
        // Set up the X pieces at the bottom of the board
        for (int i = 7; i > 4; --i) {
            for (int j = 7; j >= 0; --j) {
                if ((i + j) % 2 == 1)
                    board[i][j] = players[0];
                else
                    board[i][j] = emptySpot;
            }
        }
        currentPlayer = startingPlayer;
    }

    /**
     * Constructs the game class and assigns the standard starting player index of 0 - X.
     */
    public Game() {
        this(0);
    }

    /**
     * Checks whether a given coordinate is valid
     * @param coordinate the coordinate to be checked
     * @return true is the coordinate is valid; false if not
     */
    public boolean isCoordinateValid(Coordinate coordinate) {
        int row = coordinate.y;
        int col = coordinate.x;
        return row >= 0 && row <= 7 && col >= 0 && col <= 7;
    }

    /**
     * Checks whether a given piece (of the current player) can jump right now
     * @param coordinate The coordinate of the checker piece
     * @return true if piece can jump; false otherwise
     */
    public boolean canJump(Coordinate coordinate) {
        int row = coordinate.y;
        int col = coordinate.x;
        int yDirection = (currentPlayer == 0) ? -1 : 1;
        // Check if coordinates are valid
        if (!isCoordinateValid(coordinate)) return false;
        // Check if current player piece selected
        if (board[row][col] != players[currentPlayer]) return false;
        // Check whether there is enough vertical space to jump
        if (row + 2 * yDirection < 0 || row + 2 * yDirection > 7) return false;
        // Check if we can jump on the left
        if (col - 2 >= 0 && board[row + yDirection][col - 1] == players[(currentPlayer + 1) % 2] && board[row + 2 * yDirection][col - 2] == emptySpot) return true;
        // Check if we can jump on the right
        return col + 2 <= 7 && board[row + yDirection][col + 1] == players[(currentPlayer + 1) % 2] && board[row + 2 * yDirection][col + 2] == emptySpot;
    }

    /**
     * Checks whether a given piece can move
     * @param coordinate the coordinate representing the checker piece
     * @return whether a piece has available moves
     */
    public boolean canMove(Coordinate coordinate) {
        int row = coordinate.y;
        int col = coordinate.x;
        int yDirection = (currentPlayer == 0) ? -1 : 1;
        // Check whether coordinates are valid
        if(!isCoordinateValid(coordinate)) return false;
        // Check whether current player piece is selected
        if (board[row][col] != players[currentPlayer]) return false;
        // Check if there is space in front of the piece
        if (row + yDirection < 0 || row + yDirection > 7) return false;
        // Check whether this piece can jump
        if (canJump(coordinate)) return true;
        // Check if other pieces have to jump
        for (int i = 0; i < board.length; ++i) {
            for (int j = 0; j < board[0].length; ++j) {
                if (canJump(new Coordinate(j, i))) return false;
            }
        }
        // Check left diagonal
        if (col - 1 >= 0 && board[row + yDirection][col - 1] == emptySpot) return true;
        // Check the right diagonal
        return col + 1 <= 7 && board[row + yDirection][col + 1] == emptySpot;
    }

    /**
     * Validates a checker move. If the starting position starts not at a checker or the checker moves to an invalid
     * position, this function will return false.
     * @param from The coordinate of the piece to be moved
     * @param to The coordinate of the location to be moved to
     * @return whether the move was valid
     */
    public boolean isMoveValid(Coordinate from, Coordinate to) {
        int yDirection = (currentPlayer == 0) ? -1 : 1;
        int yDistance = to.y - from.y;
        int xDistance = to.x - from.x;
        // Check validity of coordinates
        if (!isCoordinateValid(from) || !isCoordinateValid(to)) return false;
        // Check whether the player piece is selected
        if (board[from.y][from.x] != players[currentPlayer]) return false;
        // Check whether piece can move
        if (!canMove(from)) return false;
        // Check whether the move spot is empty
        if (board[to.y][to.x] != emptySpot) return false;
        // If this is a jump, check whether end coordinates are correct
        if (canJump(from) && from.y + 2 * yDirection == to.y && xDistance * xDistance == 4) return true;
        // Check whether this is a diagonal jump forward
        return yDistance == yDirection && xDistance * xDistance == 1;
    }

    /**
     * Checks for a game over state. The game is over if the current player can no longer make any moves.
     * @return true for game over, false for game not over
     */
    public boolean isGameOver() {
        for (int i = 0; i < board.length; ++i) {
            for (int j = 0; j < board[0].length; ++j) {
                // If a piece of the current player can move, then game is not over
                if (players[currentPlayer] == board[i][j] && canMove(new Coordinate(j, i))) return false;
            }
        }
        return true;
    }

    /**
     * Moves the given checker to a new position. Will do nothing if the positions are invalid, so be careful.
     * @param from the coordinate of the chosen piece
     * @param to the coordinate of the spot to be moved to
     */
    public void move(Coordinate from, Coordinate to) {
        // If move is invalid, do not move
        if (!isMoveValid(from, to)) return;
        // If this was a jump, do jump movement and check whether we can jump again
        if (canJump(from)) {
            int yDirection = (currentPlayer == 0) ? -1 : 1;
            int xDirection = (from.x > to.x) ? -1 : 1;
            board[from.y + yDirection][from.x + xDirection] = emptySpot;
            board[to.y][to.x] = board[from.y][from.x];
            board[from.y][from.x] = emptySpot;
            if (!canJump(to))
                currentPlayer = (currentPlayer + 1) % 2;
        } else { // Do simple forward movement
            board[to.y][to.x] = board[from.y][from.x];
            board[from.y][from.x] = emptySpot;
            currentPlayer = (currentPlayer + 1) % 2;
        }
    }

    /**
     * Returns the current player index
     * @return 0 for player X, 1 for player O
     */
    public int getCurrentPlayer() {
        return currentPlayer;
    }

    /**
     * Returns the selected player character. Be careful, there are only two players. Invalid indexes will result in a
     * crash, so check bounds before using this function.
     * @param playerIndex 0 for player 1, 1 for player 2.
     * @return character representing player
     */
    public char getPlayer(int playerIndex) {
        return players[playerIndex];
    }

    /**
     * Returns the game board
     * @return the game board
     */
    public char[][] getBoard() {
        return board;
    }
}

Main.java:

package core;

import ui.TerminalUI;

public class Main {
    private Game game;
    private TerminalUI ui;

    /**
     * Parses the string input into two usable coordinates. The format should be something like 3a-4b
     * @param input the input string
     * @return two coordinates, one for source and the other for destination
     */
    private Coordinate[] parseInput(String input) {
        String[] parts = input.split("-");
        Coordinate[] toReturn = new Coordinate[2];

        int row, col;
        String str;
        for (int i = 0; i < 2; ++i) {
            str = parts[i];
            row = 8 - Integer.parseInt(str.substring(0, 1));
            switch(str.charAt(1)) {
                case 'a':
                    col = 0;
                    break;
                case 'b':
                    col = 1;
                    break;
                case 'c':
                    col = 2;
                    break;
                case 'd':
                    col = 3;
                    break;
                case 'e':
                    col = 4;
                    break;
                case 'f':
                    col = 5;
                    break;
                case 'g':
                    col = 6;
                    break;
                case 'h':
                    col = 7;
                    break;
                default:
                    col = 0;
            }
            toReturn[i] = new Coordinate(col, row);
        }

        return toReturn;
    }

    /**
     * Initializes the class
     */
    public Main() {
        game = new Game();
        ui = new TerminalUI();
    }

    /**
     * Represents the primary game loop
     */
    public void run() {
        // Print the game board
        ui.printBoard(game.getBoard());
        // Print game start message
        ui.printGameBegin(game.getPlayer(game.getCurrentPlayer()));
        String input = ui.promptMove();
        Coordinate coordinates[] = parseInput(input);
        game.move(coordinates[0], coordinates[1]);
        while(!game.isGameOver()) {
            ui.printBoard(game.getBoard());
            ui.printPlayerTurn(game.getPlayer(game.getCurrentPlayer()));
            input = ui.promptMove();
            coordinates = parseInput(input);
            game.move(coordinates[0], coordinates[1]);
        }
        ui.printWin(game.getPlayer((game.getCurrentPlayer() + 1) % 2));
    }

    public static void main(String[] args) {
        Main main = new Main();
        main.run();
    }
}

TerminalUI.java:

package ui;

import java.util.Scanner;

public class TerminalUI {
    private Scanner in;

    /**
     * Constructs the class and its attributes
     */
    public TerminalUI() {
        in = new Scanner(System.in);
    }

    /**
     * Prints the board state of the game via the board parameter.
     * @param board 8x8 array representing the current state of the game board
     */
    public void printBoard(char[][] board) {
        for (int i = 0; i < board.length; ++i) {
            System.out.print(8 - i + " |");
            for (int j = 0; j < board[0].length; ++j) {
                System.out.printf(" %c |", board[i][j]);
            }
            System.out.println();
        }
        System.out.println("    a   b   c   d   e   f   g   h");
    }

    /**
     * Prints a message telling the user which player's turn it is now
     * @param player the character representing the player
     */
    public void printPlayerTurn(char player) {
        System.out.println("Player " + player + " - your turn.");
    }

    /**
     * Prints a message telling the user that the game has started and which player's turn it is
     * @param player the character representing the player
     */
    public void printGameBegin(char player) {
        System.out.print("Begin Game. ");
        printPlayerTurn(player);
    }

    /**
     * Prints a win message and congratulates the winning player
     * @param player the character representing the player who won
     */
    public void printWin(char player) {
        System.out.println("Player " + player + " Won the Game!");
    }

    /**
     * Prints the given string. To print a newline, you should include the \n escape sequence in the string.
     * @param text String representing the text to be printed
     */
    public void print(String text) {
        System.out.print(text);
    }

    /**
     * Prints a newline in the output
     */
    public void newline() {
        System.out.println();
    }

    /**
     * Prompts the user for input with a given message
     * @param text Prompt message
     * @return input received from user
     */
    public String prompt(String text) {
        System.out.println(text);
        return in.nextLine();
    }

    /**
     * Prompts the user to input the next move
     * @return input received from user
     */
    public String promptMove() {
        String prompt = "Choose a cell position of piece to be moved and the new position (e.g., 3a-4b):";
        return prompt(prompt);
    }
}

Coordinate.java:

package core;

public class Coordinate {
    int x;
    int y;

    /**
     * Initializes this structure with the given coordinates
     * @param x the X coordinate
     * @param y the Y coordinate
     */
    public Coordinate(int x, int y) {
        this.x = x;
        this.y = y;
    }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ In parseInput(), please assign col = str.charAt(1) - 'a'; instead of the giant switch. Possibly it needs some (int) casting to retrieve the ASCII value of the char. \$\endgroup\$
    – J_H
    Commented Apr 19 at 15:01

2 Answers 2

5
\$\begingroup\$

Use Java 16 Records

Coordinate is a typical transparent data-carrier.

record Coordinate(int row, int col) { }

Apply Information expert principle

Coordinate type should own the logic on how to parse it and validate its properties, not an external class.

record Coordinate(int row, int col) {
    public static boolean isValid(int row, int col) { ... }
    public static Coordinate[] parseCoordinates(String input) { ... }
}

Use Switch expressions

Switch expressions are way more readable and compact than switch statements.

int col = switch (coordinate.charAt(1)) {
    case 'a' -> 0;
    case 'b' -> 1;
    case 'c' -> 2;
    case 'd' -> 3;
    case 'e' -> 4;
    case 'f' -> 5;
    case 'g' -> 6;
    case 'h' -> 7;
    default  -> throw new IllegalArgumentException(); // not 'default -> 0;'
};

Also, avoid returning a valid value from the default case. Make your switch exhaustive, and if default case required (for instance, with enums it's not the case) throw an exception.

And, by the way, it's not necessary to use switch in to parse user input, intead you can leverage the face that char is numeric type (see code below).

Fail Fast

Avoid propagating objects with invalid state.

It means that ideally Game should receive from the user only valid coordinates, to achieve that you need to include a validation step into parsing logic.

public record Coordinate(int row, int col) {
    private static final Pattern VALID_INPUT = Pattern.compile("([1-8][a-h])-([1-8][a-h])");
    private static final char FIRST_CHARACTER = 'a';

    public static boolean isValid(int row, int col) {
        return row >= 0 && row <= 7
            && col >= 0 && col <= 7;
    }
    
    public static Coordinate[] parseCoordinates(String input) {
        var matcher = VALID_INPUT.matcher(input);
        if (!matcher.matches()) throw new IllegalArgumentException();
        
       return new Coordinate[]{
           parse(matcher.group(1)),
           parse(matcher.group(2))
       };
    }
    
    private static Coordinate parse(String coordinate) {
        int row = 8 - Character.getNumericValue(coordinate.charAt(0));
        int col = coordinate.charAt(1) - FIRST_CHARACTER; // equivalent of switch shown above

        return new Coordinate(col, row);
    }
}

Regular expression "([1-8][a-h])-([1-8][a-h])" takes care of validating user input:

  • every group ([1-8][a-h]) matches a two-character string;
  • 1-8 matches a single character between '1' and '8';
  • a-h matches a single character in the range between 'a' and 'h'.

Also consider handling the exception (you can define a custom exception type instead of using IllegalArgumentException) caused by malformed user input.

That would improve user experience by giving them the opportunity to retry instead of throwing stack trace at them.

Don't expose encapsulated data

Object-orientation is about exposing behavior, not data.

There's no proper encapsulation in the Game class.

This is not quite right:

public char[][] getBoard() { return board; }
ui.printBoard(game.getBoard());

Exposing the array board is wrong for several reasons:

  • it's a mutable object holding internal state exposed via public method;
  • usage of char[][] array is an implementation detail of the Game class;
  • method printBoard() is aware of how to dial with the internal data structure of the Game class, i.e. you created a tight coupling between TerminalUI and Game.

What you should do instead? - Apply the Information expert principle.

Instead of making TerminalUI aware of how the string representation of the board should look like, move this logic into toString() method of the Game class.

Inline Comments in the code is a smell

When you need to place a comment in the code in order to reason about it, it means that something is lacking. The essence of what you described in the comment should be expressed through proper method names and variable names.

Example:

public boolean isGameOver() {
    for (int i = 0; i < board.length; ++i) {
        for (int j = 0; j < board[0].length; ++j) {
            // If a piece of the current player can move, then game is not over
            if (players[currentPlayer] == board[i][j] && canMove(new Coordinate(j, i))) return false;
        }
    }
    return true;
}

If you extract the condition into a separate method with a self-explanatory name, there would be no need for the comment.

When you can not simplify the code by extracting methods (you tried, and It's still messy), consider introducing an abstraction.

Document behavior / Avoid useless doc comments

Documentation comment should be used to provide information on the intention of the code element.

You can explain what problem it solves (if it's not immediately obvious from the name), provide usage examples if needed, specify valid ranges of expected parameters, describe side effects, behavior in edge cases, constraints and assumptions, and other subtleties which reader of the code can not deduce from the name.

But you should not include redundant and obvious information and things like implementation details.

Example:

/**
 * Constructs the class and its attributes
 */
public TerminalUI() {
    in = new Scanner(System.in);
}

That's an example of obvious information which is not helpful for the user of your code.

Avoid using different names for the same concept

Example: x, y and row, col.

Don't alternate between different names, that causes confusion for the reader. Choose the one that fits better.

Define static constants

Turn the strings scatted across the methods in the UI into compile time constants:

class TerminalUI {
    private static final String NEXT_TURN_PROMPT = "Player %s - your turn.";

    // ...

    public void printPlayerTurn(Player player) {
        System.out.println(NEXT_TURN_PROMPT.formatted(player));
    }
}

Make use of String.formatted() when you need to inject data into these static final strings.

The name of a Code element should reflect its Purpose

Class Main based on its contents should be named GameRunner or GameManger.

My advise: rename Main to GameRunner and create and create class Main hosting only main() method in a sepate file.

Consider introducing abstractions to manage code complexity

I would advice to consider defining some abstractions that would enable simplification of the business logic.

NOTE: that all the code in the answer should be considered as a source of inspiration, not as the only proper implementation

So, to begin with, think about defining an enum named Player (instead of juggling with 1 and 0, X and O) and a nested class Cell. The board would became an array of cells.

public class Game {
    
    public enum Player {
        WHITE("X"), BLACK("O");
        
        private final String symbol; 
        
        Player(String symbol) {
            this.symbol = symbol;
        }
        
        @Override
        public String toString() {
            return symbol;
        }
    }

    private static final Cell BLACK_EMPTY_CELL = new EmptyCell(Cell.Color.BLACK);
    private static final Cell WHITE_EMPTY_CELL = new EmptyCell(Cell.Color.WHITE);
    private static final Cell CELL_WITH_WHITE_PIECE = new OccupiedCell(Player.WHITE);
    private static final Cell CELL_WITH_BLACK_PIECE = new OccupiedCell(Player.WHITE);
    private final Cell[][] board;
    private Player currentPlayer = Player.WHITE;

    // other stuff
    
    interface Cell {
        boolean isEmpty();
        boolean isOccupiedBy(Player player);
        
        enum Color {
            BLACK("■"), WHITE(" "); // symbols can be used in the UI to represent unoccupied cells
            
            private final String symbol;
            
            Color(String symbol) {
                this.symbol = symbol;
            }
            
            @Override
            public String toString() {
                return symbol;
            }
        }
    }
    
    private record EmptyCell(Color color) implements Cell {
        
        @Override
        public boolean isEmpty() {
            return true;
        }
        
        @Override
        public boolean isOccupiedBy(Player player) {
            return false;
        }
        
        @Override
        public String toString() {
            return color.toString();
        }
    }
    
    private record OccupiedCell(Player player) implements Cell {
        
        @Override
        public boolean isEmpty() {
            return false;
        }
        
        @Override
        public boolean isOccupiedBy(Player player) {
            return this.player == player;
        }
        
        @Override
        public String toString() {
            return player.toString();
        }
    }

}

If you have a requirement to display 0 or 1 on the UI when referring to a player, use Enum.ordinal() method to get the position of the enum constant.

Note, that there would be only created 4 (!) instances of the Cell type. They are stateless and we can reuse them.

Now, let's first refactor the constructor by moving all the heavy logic from it into a factory method:

public class Game {
    
    // ...
    
    private Game(Cell[][] board) {
        this.board = board;
    }
    
    public static Game create() {
        var board = new Cell[8][8];
        
        setWhitePieces(board);
        setBlackPieces(board);
        setEmptyCells(board);
        
        return new Game(board);
    }
    
    private static void setWhitePieces(Cell[][] board) {
        for (int i = 0; i < 3; ++i) {
            for (int j = 0; j < 8; ++j) {
                if (isBlackCell(i, j)) {
                    board[i][j] = CELL_WITH_WHITE_PIECE;
                }
            }
        }
    }
    
    private static void setBlackPieces(Cell[][] board) {
        for (int i = 7; i > 4; --i) {
            for (int j = 7; j >= 0; --j) {
                if (isBlackCell(i, j)) {
                    board[i][j] = CELL_WITH_BLACK_PIECE;
                }
            }
        }
    }
    
    private static void setEmptyCells(Cell[][] board) {
        for (int i = 0; i < 8; ++i) {
            for (int j = 0; j < 8; ++j) {
                if (board[i][j] != null) continue;
                
                if (!isBlackCell(i, j)) {
                    board[i][j] = BLACK_EMPTY_CELL;
                } else {
                    board[i][j] = WHITE_EMPTY_CELL;
                }
            }
        }
    }
    
    private static boolean isBlackCell(int i, int j) {
        return (i + j) % 2 == 1;
    }
}

Pay attention to the fact that helper methods setWhitePieces, setBlackPieces and setEmptyCells correspond to the sections of your initial code sprinkled with inline comments.

Now, let's address your question regarding reducing complexity of methods like canMove, canJump, ect.

As an example, let's refactor canJump:

public class Game {
    
    // ...
    
    private boolean canJump(int row, int col) {
        if (!isOccupiedByCurrentPlayer(row, col)) return false;
        
        return canCapturePiece(row, col, ColumnDirection.LEFT)
            || canCapturePiece(row, col, ColumnDirection.RIGHT);
    }
    
    private boolean canCapturePiece(int row, int col, 
                                    ColumnDirection direction) {
        return hasEnemyPiece(row, col, direction)
            && canJump(row, col, direction);
    }
    
    private boolean hasEnemyPiece(int row, int col, ColumnDirection colDirection) {
        int enemyRow = row + rowDirection();
        int enemyCol = col + colDirection.toInt();
        
        return Coordinate.isValid(enemyRow, enemyCol)
            && isOccupiedByEnemy(enemyRow, enemyCol);
    }
    
    private boolean canJump(int row, int col, 
                            ColumnDirection colDirection) {
        int targetRow = row + 2 * rowDirection();
        int targetCol = col + 2 * colDirection.toInt();
        
        return Coordinate.isValid(targetRow, targetCol) 
            && isEmpty(targetRow, targetCol);
    }
    
    private int rowDirection() {
        return currentPlayer == Player.WHITE ? -1 : 1;
    }
    
    private boolean isEmpty(int row, int col) {
        return board[row][col].isEmpty();
    }
    
    private boolean isOccupiedByCurrentPlayer(int row, int col) {
        return board[row][col].isOccupiedBy(currentPlayer);
    }
    
    private boolean isOccupiedByEnemy(int row, int col) {
        return !board[row][col].isEmpty()
            &&  board[row][col].isOccupiedBy(currentPlayer);
    }
    
    private enum ColumnDirection {
        LEFT(-1), RIGHT(1);
        private int ratio;
        
        ColumnDirection(int ratio) {
            this.ratio = ratio;
        }
        
        public int toInt() {
            return ratio;
        }
    }
}

Note, parameters row and col are used directly in this instead of Coordinate.

Coordinate type is useful and convenient when we are parsing user input and for interacting with the Game instance from the outside, namely through method move:

public void move(Coordinate from, Coordinate to)

But then inside Game class while passing around instances of Coordinate you're introding local variables representing rows and column and then again constructing new Coordinate instances just to invoke internal methods (look at canMove method). That means that in these cases usage of this type is no longer helpful.

Note that what I told about Coordinate is an observation made base the code you shared, not a strong recomendation. After introducing a series of changes it might still hold true, or might not.
The point is that you need to develop a habit of a making small incremental changes while evolving your disign and constanly evaluating the code elements you're work with at the moment.
The driving motivation behind every design change should be your understanding of the value this change might bring.
It's normal to revert the change if it doesn't improve the design. It's also normal for individual components of the system to be discarded when they no longer serve their purpose or don't play well with each other. In such cases, it may be necessary to introduce different abstractions to facilitate the addition of a new feature.
The abstractions I proposed to add (Cell and Player) are also not something set in stone. Introducing them into the code is a feasible and relatively small change which you can immediately leverage to improve the code quality. After you finish with this change, evaluate your design. You might come to a conclusion that you need to alter them in someway, or you might decide to introduce Board abstraction and incorporate them into it.

Here's how canMove can be implemented based on the functionality intoduced earlier:

public boolean canMove(int row, int col) {
    if (!isOccupiedByCurrentPlayer(row, col)) return false;
    if (anyCanJump()) return false;
    
    return canMove(row, col, ColumnDirection.LEFT)
        || canMove(row, col, ColumnDirection.RIGHT);
}

private boolean anyCanJump() {
    boolean canJump = false;
    for (int row = 0; row < board.length; row++) {
        for (int col = 0; col < board[0].length; col++) {
            canJump = canJump(row, col);
            if (canJump) break;
        }
    }
    return canJump;
}

private boolean canMove(int row, int col, ColumnDirection colDirection) {
    int targetRow = row + 2 * rowDirection();
    int targetCow = col + 2 * colDirection.toInt();
    
    return Coordinate.isValid(targetRow, targetCow)
        && isEmpty(targetRow, targetCow);
}
\$\endgroup\$
11
  • \$\begingroup\$ Very good analysis, I thank you so very much! There is one piece that I find confusing: is there any way to avoid canJump and canCapturePiece from relying so much on each other? The interdependency makes the code very confusing and I feel like there could be an infinite loop. Additionally, I noticed that canJump does not take into account whether there is the space to jump. On the other hand, now that the coordinates are always on the board, we do not have to check whether we are jumping on the board. \$\endgroup\$ Commented Apr 21 at 19:08
  • \$\begingroup\$ @ViceroyFaust "does not take into account whether there is the space" - it does this check, examine this method canJump(int row, int col, ColumnDirection colDirection) \$\endgroup\$ Commented Apr 22 at 22:21
  • \$\begingroup\$ @ViceroyFaust In the game of checkers, opponent's pieces are being captured by jumping over them. I.e. the act of just is an attack on the enemy piece. So I'm not quite getting your idea about decoupling canJump and canCapturePiece. Maybe you meant canMove and canJump? I've just added implementation of canMove (at the very end of the answer) \$\endgroup\$ Commented Apr 22 at 22:35
  • \$\begingroup\$ @ViceroyFaust By the way, your implementation doesn't allow capturing multiple pieces (jumping over multiple pieces arranged in chain with the gaps), which is one of the key features of the Checkers. Is that an intentional simplification? i.sstatic.net/HSqUO.jpg \$\endgroup\$ Commented Apr 22 at 23:14
  • \$\begingroup\$ it does allow for it, but you have to manually jump first. It won't let you move after jumping if you can still jump. I found it very hard to implement these rules, so I did not figure out how to calculate full jump paths. \$\endgroup\$ Commented Apr 23 at 6:10
3
\$\begingroup\$

Do you know these kinds of Memes?

enter image description here

The code to build the board is somehow the same. We already know how the board has to look like. Since it is not dynamic, we don't need loops and any other logic to build the board:

this.board = new char[][]{
    {'_', 'O', '_', 'O', '_', 'O', '_', 'O'},
    {'O', '_', 'O', '_', 'O', '_', 'O', '_'},
    {'_', 'O', '_', 'O', '_', 'O', '_', 'O'},
    {'_', '_', '_', '_', '_', '_', '_', '_'},
    {'_', '_', '_', '_', '_', '_', '_', '_'},
    {'X', '_', 'X', '_', 'X', '_', 'X', '_'},
    {'_', 'X', '_', 'X', '_', 'X', '_', 'X'},
    {'X', '_', 'X', '_', 'X', '_', 'X', '_'}
};

Now the reader of the code don't have to get his head around three nested for-loops. Instead, the reader can see immediately the board and its pieces.


I do like @Alexander Ivanchenko's review. But one and for me the most important abstraction is missing: Board.

A Board knows if a Coordinate is valid and if the Coordinate is pointing on a piece of the first player, the second player or on an empty cell.

Just as an example:

class Game {
    // ...
    
    public boolean canMove(Coordinate coordinate) {
        int yDirection = (currentPlayer == 0) ? -1 : 1;

        // Check whether coordinates are valid
        if (!board.isValid(coordinate)) return false;

        // Check whether current player piece is selected
        if (currentPlayer == 0 && board.isSecondPlayerOn(coordinate)) {
            return false;
        }

        if (currentPlayer == 1 && board.isFirstPlayerOn(coordinate)) {
            return false;
        }

        // Check if there is space in front of the piece
        if (!board.isValid(coordinate.y(0, yDirection))) {
            return false;
        }

        // Check whether this piece can jump
        if (canJump(coordinate)) {
            return true;
        }

        // Check if other pieces have to jump
        if (board.any(this::canJump)) {
            return false;
        }

        // Check left diagonal
        if (board.isValid(coordinate.x(-1)) && board.isEmptyAt(coordinate.x(-1).y(yDirection)))) {
            return true;
        }

        // Check the right diagonal
        return board.isValid(coordinate.x(1))) &&board.isEmptyAt(coordinate.x(1).y(yDirection));
    }
   
    // ...
}
public class Board {
    private final char[][] board;
    private final char firstPlayer = 'X';
    private final char secondPlayer = 'O';
    private final char emptySpot = '_';

    Board() {
        this.board = new char[][]{
                {'_', 'O', '_', 'O', '_', 'O', '_', 'O'},
                {'O', '_', 'O', '_', 'O', '_', 'O', '_'},
                {'_', 'O', '_', 'O', '_', 'O', '_', 'O'},
                {'_', '_', '_', '_', '_', '_', '_', '_'},
                {'_', '_', '_', '_', '_', '_', '_', '_'},
                {'X', '_', 'X', '_', 'X', '_', 'X', '_'},
                {'_', 'X', '_', 'X', '_', 'X', '_', 'X'},
                {'X', '_', 'X', '_', 'X', '_', 'X', '_'}
        };
    }

    boolean isFirstPlayerOn(Coordinate coordinate) {
        return board[coordinate.y()][coordinate.x()] == firstPlayer;
    }

    boolean isSecondPlayerOn(Coordinate coordinate) {
        return board[coordinate.y()][coordinate.x()] == secondPlayer;
    }

    boolean isValid(Coordinate coordinate) {
        return coordinate.y() >= 0 &&
                coordinate.y() <= 7 &&
                coordinate.x() >= 0 &&
                coordinate.x() <= 7;
    }

    boolean any(Predicate<Coordinate> predicate) {
        for (int i = 0; i < board.length; ++i) {
            for (int j = 0; j < board[0].length; ++j) {
                if (predicate.test(new Coordinate(j, i))) {
                    return false;
                }
            }
        }

        return true;
    }

    boolean isEmptyAt(Coordinate coordinate) {
        return board[coordinate.y][coordinate.x] == emptySpot;
    }
}

From there it can be further simplified.

\$\endgroup\$
4
  • \$\begingroup\$ @Alexander Ivanchenko suggested that I put the coordinate checking system inside the coordinate itself. However, I like your approach more, since the coordinate points to the board. Therefore it makes more sense for the board to know what's a right spot and what's a wrong spot. After all, in real life you look at a checker's board to check these things haha. \$\endgroup\$ Commented Apr 21 at 10:52
  • \$\begingroup\$ In Domain-Driven-Design it is common pattern to put the validation inside a “Value Object” like Coordinate. But the downside in our scenario is, that a Coodrinate and the Board/ (or the Game) have shared knowledge about the minimum and maximum of the width and the height of the board. When this values will never change, this will never be a problem. But if they will change, because you decide to make the board bigger, you will have to change multiple classes because of the shared knowledge. \$\endgroup\$
    – Roman
    Commented Apr 21 at 11:15
  • \$\begingroup\$ Another solution could be that there is something like a RuleEngine or the Game is behaving like one. And the “Rule Engine” knows the dimensions of the board. Since it knows the dimensions it can create a Board with these dimensions and validates a Coordinates if it is inside these. As you can see, there are multiple solutions even for this “simple” problem. You did a great job. Have fun! :) \$\endgroup\$
    – Roman
    Commented Apr 21 at 11:22
  • \$\begingroup\$ What does method any() do? I have not learned the predicate class, so I do not understand what predicate.test(...) does there. \$\endgroup\$ Commented Apr 21 at 14:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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