5
\$\begingroup\$

I have 3 classes to create a simple checkers game move validator. The entry point is checkers.py and is mostly complete with a few things hard-coded. For example, the auto-generated board (if none is given) starts with the upper left square as white, whereas if we are given a board, we are assuming the upper left square is black (black = legal square we can move to, white squares we cannot move to). Also when validating a move, we are only considering the case where the piece we are moving can move diagonally downwards (e.g. the piece started at the top of the board and is not a king).

I feel like my design is not very good and there is too much hard-coding, so I want to get some feedback before I continue any further. Please let me know what improvements I can make.

checkers.py

from piece import Piece, Side
from validator import Validator


class Checkers:
    '''
    Checkers class can either generate an empty board or take in a board. The two different checkers boards available
    are either top left square black or white and then they alternate. I have top left color as black hard coded ri
    ght now (if a board is given). If a board is not given, the board is harded coded for top left color as white.
    '''
    def __init__(self, board=None):
        self.BOARD_ROWS, self.BOARD_COLS = 8, 8
        self.legal_spaces = set()
        if not board:
            self.board = [[None for _ in range(self.BOARD_COLS)] for _ in range(self.BOARD_ROWS)]
            for row in range(self.BOARD_ROWS):
                if (row % 2) == 0:
                    for col in range(1, self.BOARD_COLS, 2):
                        self.legal_spaces.add((row, col))
                        if 0 <= row <= 2:
                            self.board[row][col] = Piece(Side.RED)
                        elif 5 <= row <= 7:
                            self.board[row][col] = Piece(Side.BLACK)
                else:
                    for col in range(0, self.BOARD_COLS, 2):
                        self.legal_spaces.add((row, col))
                        if 0 <= row <= 2:
                            self.board[row][col] = Piece(Side.RED)
                        elif 5 <= row <= 7:
                            self.board[row][col] = Piece(Side.BLACK)
        elif Validator.validate_board(board):
            self.board = board
            self.generate_legal_spaces('B')
            for row in range(self.BOARD_ROWS):
                for col in range(self.BOARD_COLS):
                    match board[row][col]:
                        case 'R':
                            board[row][col] = Piece(Side.RED)
                        case 'B':
                            board[row][col] = Piece(Side.BLACK)
                        case _:
                            board[row][col] = None
        else:
            raise ValueError('Invalid Board.')

    def generate_legal_spaces(self, top_left_color):
        if top_left_color not in ('B', 'W'):
            raise ValueError('Incorrect top left color provided')
        for row in range(self.BOARD_ROWS):
                if top_left_color == 'B':
                    if (row % 2 == 0):
                        self.legal_spaces.update([(row, 0), (row, 2), (row, 4), (row, 6)])
                    else:
                        self.legal_spaces.update([(row, 1), (row, 3), (row, 5), (row, 7)])
                else:
                    if (row % 2 == 0):
                        self.legal_spaces.update([(row, 1), (row, 3), (row, 5), (row, 7)])
                    else:
                        self.legal_spaces.update([(row, 0), (row, 2), (row, 4), (row, 6)])

    def __str__(self):
        output = ''
        for row in range(self.BOARD_ROWS):
            for col in range(self.BOARD_COLS):
                square = self.board[row][col]
                if not square:
                    output += '_'
                else:
                    output += square.side.value
                if col != self.BOARD_COLS - 1:
                    output += ' '
            if row != self.BOARD_ROWS - 1:
                output+= '\n'
        return output

validator.py

from piece import Side


class Validator:
    @staticmethod
    def validate_board(board):
        BOARD_ROWS, BOARD_COLS = 8, 8
        invalid_vectors = [(0, 1), (0, -1), (1, 0), (-1, 0)]
        if len(board) != BOARD_ROWS:
            return False

        def occupied(location):
            row, col = location[0], location[1]
            if (0 <= row <= 7) and (0 <= col <= 7) and board[row][col]:
                return True
            return False

        for row in range(BOARD_ROWS):
            for col in range(BOARD_COLS):
                curr_location = (row, col)
                if occupied(curr_location):
                    for invalid_vector in invalid_vectors:
                        invalid_candidate = tuple([sum(x) for x in zip(curr_location, invalid_vector)])
                        if occupied(invalid_candidate):
                            return False
        return True

    @staticmethod
    def valid_location(legal_spaces, location):
        row, col = location[0], location[1]
        if not (0 <= row <= 7) or not (0 <= col <= 7):
            return False
        if location not in legal_spaces:
            return False
        return True

    @staticmethod
    def occupied(board, location):
        return not Validator.unoccupied(board, location)

    @staticmethod
    def unoccupied(board, location):
        row, col = location[0], location[1]
        if not (0 <= row <= 7) or not (0 <= col <= 7):
            return False
        return not board[row][col]

    @staticmethod
    def occupied_side(board, location, side):
        # is square within board and occupied by the color given in side
        row, col = location[0], location[1]
        if not (0 <= row <= 7) or not (0 <= col <= 7):
            return False
        piece = board[row][col]
        if piece and piece.side == side:
            return True
        return False

    @staticmethod
    def validate_move(board, legal_spaces, start, end):
        '''
        Only handles the case where starting piece is red and not a king,
        so we can only move diagonally downwards
        '''
        if not Validator.valid_location(legal_spaces, start) or not \
            Validator.valid_location(legal_spaces, end):
            raise ValueError('Provided start or end location is out of bounds')
        start_row, start_col = start[0], start[1]
        end_row, end_col = end[0], end[1]
        # check if we are ending at unoccupied square
        if Validator.occupied(board, end):
            return False
        piece = board[start_row][start_col]
        villain_side = Side.BLACK if (piece.side == Side.RED) else Side.RED
        move_vectors = [(1, -1), (1, 1)]
        jump_vectors = [{'villain_vector': (1, -1), 'captured_vector': (2, -2)}, \
            {'villain_vector': (1, 1), 'captured_vector': (2, 2)}]
        # can we get to end by just moving
        for move_vector in move_vectors:
            move_candidate = tuple(sum(x) for x in zip(start, move_vector))
            if Validator.unoccupied(board, move_candidate) and move_candidate == end:
                return True
        # cannot get to end by move, need to jump
        return Validator.validate_jump(board, legal_spaces, jump_vectors, start, end, villain_side)

    @staticmethod
    def validate_jump(board, legal_spaces, jump_vectors, curr, target, villain_side):
        if curr == target:
            return True
        if curr in legal_spaces:
            for jump_vector in jump_vectors:
                villain_candidate = tuple(sum(x) for x in zip(curr, jump_vector['villain_vector']))
                jump_candidate = tuple(sum(x) for x in zip(curr, jump_vector['captured_vector']))
                if Validator.occupied_side(board, villain_candidate, villain_side) and Validator.unoccupied(board, jump_candidate):
                    villain_row, villain_col = villain_candidate[0], villain_candidate[1]
                    captured_piece = board[villain_row][villain_col]
                    board[villain_row][villain_col] = None
                    return Validator.validate_jump(board, legal_spaces, jump_vectors, jump_candidate, target, villain_side)
                    board[villain_row][villain_col] = captured_piece
        return False

piece.py

from enum import Enum

class Side(Enum):
    RED = 'R'
    BLACK = 'B'

class Piece:
    def __init__(self, side):
        if side not in (Side.RED, Side.BLACK):
            raise ValueError('Piece must be either red or black and be of type Side.')
        self.side = side
        self.is_king = False

    def __str__(self):
        return f'<Color: {self.side}, King: {self.is_king}>'
\$\endgroup\$

3 Answers 3

Reset to default
3
\$\begingroup\$

Documentation

The Checkers class docstring has lines that are too long. There is also a word split across 2 lines:

coded ri
ght now

This has shorter lines:

'''
Checkers class can either generate an empty board or take in a board.  The
two different checkers boards available are either top left square black
or white and then they alternate.  I have top left color as black hard
coded right now (if a board is given).  If a board is not given, the board
is hard coded for top left color as white.
'''

The PEP 8 style guide recommends adding docstrings for each class, as well as functions.

OOP

It is more common to use self instead of the class name when accessing class members, such as Validator.unoccupied in the following line:

    return not Validator.unoccupied(board, location)

You did this in the other 2 classes.

DRY

These lines are repeated 4 times in different functions in the Validator class:

row, col = location[0], location[1]
if not (0 <= row <= 7) or not (0 <= col <= 7):

Also, the same check is repeated within the line. You can factor this code out to a new function to reduce the repetition. You probably also want to use the constants (BOARD_ROWS, BOARD_COLS) instead of the magic number 7.

Simpler

In Checkers __str__, the code could be simpler if you invert the logic. Change:

if not square:
    output += '_'
else:
    output += square.side.value

to:

if square:
    output += square.side.value
else:
    output += '_'

Unreachable code

The line after the return line in the validate_jump function will never be reached and executed:

return Validator.validate_jump(board, legal_spaces, jump_vectors, jump_candidate, target, villain_side)
board[villain_row][villain_col] = captured_piece

Review the code to see if you really meant to use the line for something; otherwise, you should delete the line.

\$\endgroup\$
3
\$\begingroup\$

tuple unpack

In a few places we see

            row, col = location[0], location[1]

Prefer to unpack that tuple in the usual way:

            row, col = location
\$\endgroup\$
2
\$\begingroup\$

Complexity

Your Checkers.__init__ has a reasonably high complexity.

I would structure it as:

class Checkers:
    def __init__(self, board=None):
        if board is None:
            self._create_new_board()
        else:
            self._validate_board(board)

This signals two distinct paths for the initialization of Checkers.

Subjectively, for maintenance purposes if you never changed the arguments to it (i.e. board) you would never need to amend this method, which is good because amending the root initialization method of classes can be dangerous. Of course you might want to amend the create_new_board and validate_board methods but in my opinion that would be easier for colleagues to review.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.