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}>'
toolic
14.4k5 gold badges29 silver badges201 bronze badges
asked Oct 31, 2022 at 18:39
\$\endgroup\$

3 Answers 3

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.

answered Jan 15 at 10:38
\$\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
answered Jan 16 at 5:59
\$\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.

answered Jan 16 at 7:59
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.