# Chess game for my students

I teach programming, and I currently teach my class about inheritance, abstract classes, and mixins.

I wrote this code as an example, and I want to be sure it is as good as possible before I release it as a code example.

Few assumptions:

1. The code should only handle piece movements and not manage an entire game.
2. The code shouldn't handle special moves like en-passant, castling, or pawn promotion.
3. The code shouldn't force the king to move if another piece threatens it.
from abc import ABC, abstractmethod

class Color:
BLACK = 0
WHITE = 1

def enemy_of(color):
if color == Color.BLACK:
return Color.WHITE
return Color.BLACK

class Board:
BOARD_SIZE = (8, 8)

def __init__(self):
self.reset()

def get_square(self, row, col):
if not self.is_valid_square((row, col)):
return None
return self.board[row][col]

def set_square(self, row, col, piece):
self.board[row][col] = piece

def is_valid_square(self, square):
return (
square[0] in range(self.BOARD_SIZE[0])
and square[1] in range(self.BOARD_SIZE[1])
)

def is_empty_square(self, square):
return self.get_square(*square) is None

def _generate_first_row(self, color):
row_by_color = {Color.BLACK: 0, Color.WHITE: self.BOARD_SIZE[0] - 1}
row = row_by_color[color]

order = (Rook, Knight, Bishop, Queen, King, Bishop, Knight, Rook)
params = {'color': color, 'row': row}
return [order[i](col=i, **params) for i in range(self.BOARD_SIZE[0])]

def _generate_pawns_row(self, color):
row_by_color = {Color.BLACK: 1, Color.WHITE: self.BOARD_SIZE[0] - 2}
row = row_by_color[color]
params = {'color': color, 'row': row}
return [Pawn(col=i, **params) for i in range(self.BOARD_SIZE[0])]

def get_pieces(self, color=None):
for row in self.board:
for col in row:
if col is not None and (color is None or col.color == color):
yield col

def get_possible_moves(self, color, with_king=False):
"""Return all player's possible moves."""
pieces = self.get_pieces(color=color)
if not with_king:
pieces = [p for p in pieces if not isinstance(p, King)]

for piece in pieces:
for move in piece.get_valid_moves(self):
yield move

def reset(self):
self.board = [
self._generate_first_row(Color.BLACK),
self._generate_pawns_row(Color.BLACK),
[None] * self.BOARD_SIZE[0],
[None] * self.BOARD_SIZE[0],
[None] * self.BOARD_SIZE[0],
[None] * self.BOARD_SIZE[0],
self._generate_pawns_row(Color.WHITE),
self._generate_first_row(Color.WHITE),
]

def move(self, source, destination):
piece = self.get_square(*source)
return piece.move(board=self, destination=destination)

def __str__(self):
printable = ""
for row in self.board:
for col in row:
if col is None:
printable = printable + " ▭ "
else:
printable = printable + f" {col} "
printable = printable + '\n'
return printable

class Piece(ABC):
def __init__(self, color, row, col, **kwargs):
super().__init__(**kwargs)
self.color = color
self.row = row
self.col = col

def is_possible_target(self, board, target):
is_target_valid = board.is_valid_square(target)
is_empty_square = board.is_empty_square(target)
is_hitting_enemy = self.is_enemy(board.get_square(*target))
return is_target_valid and (is_empty_square or is_hitting_enemy)

@abstractmethod
def get_valid_moves(self, board):
pass

def get_position(self):
return self.row, self.col

def is_enemy(self, piece):
if piece is None:
return False
return piece.color == Color.enemy_of(self.color)

def move(self, board, destination):
if not self.is_possible_target(board, destination):
return False
if destination not in self.get_valid_moves(board):
return False

board.set_square(*self.get_position(), None)
board.set_square(*destination, self)
self.row, self.col = destination
return True

@abstractmethod
def __str__(self):
pass

class WalksDiagonallyMixin:
def __init__(self, **kwargs):
super().__init__(**kwargs)
if not hasattr(self, 'directions'):
self.directions = set()
self.directions.update({
(-1, -1),          (1, -1),

(-1,  1),          (1,  1),
})

class WalksStraightMixin:
def __init__(self, **kwargs):
super().__init__(**kwargs)
if not hasattr(self, 'directions'):
self.directions = set()
self.directions.update({
(0, -1),
(-1,  0),          (1,  0),
(0,  1),
})

class WalksMultipleStepsMixin:
def get_valid_moves(self, board):
for row_change, col_change in self.directions:
steps = 1
stop_searching_in_this_direction = False
while not stop_searching_in_this_direction:
new_row = self.row + row_change * steps
new_col = self.col + col_change * steps
target = (new_row, new_col)
is_valid_target = self.is_possible_target(board, target)
if is_valid_target:
yield target
steps = steps + 1
is_hit_enemy = self.is_enemy(board.get_square(*target))
if not is_valid_target or (is_valid_target and is_hit_enemy):
stop_searching_in_this_direction = True

class Pawn(Piece):
DIRECTION_BY_COLOR = {Color.BLACK: 1, Color.WHITE: -1}

def __init__(self, **kwargs):
super().__init__(**kwargs)
self.moved = False
self.forward = self.DIRECTION_BY_COLOR[self.color]

def _get_regular_walk(self):
src_row, src_col = self.get_position()
return (src_row + self.forward, src_col)

def _get_double_walk(self):
src_row, src_col = self.get_position()
return (src_row + self.forward * 2, src_col)

def _get_diagonal_walks(self):
src_row, src_col = self.get_position()
return (
(src_row + self.forward, src_col + 1),
(src_row + self.forward, src_col - 1),
)

def is_possible_target(self, board, target):
is_valid_move = board.is_valid_square(target)
is_step_forward = (
board.is_empty_square(target)
and target == self._get_regular_walk()
)
is_valid_double_step_forward = (
board.is_empty_square(target)
and not self.moved
and target == self._get_double_walk()
and self.is_possible_target(board, self._get_regular_walk())
)
is_hitting_enemy = (
self.is_enemy(board.get_square(*target))
and target in self._get_diagonal_walks()
)
return is_valid_move and (
is_step_forward or is_valid_double_step_forward or is_hitting_enemy
)

def move(self, **kwargs):
is_success = super().move(**kwargs)
self.moved = True
return is_success

def get_valid_moves(self, board):
targets = (
self._get_regular_walk(),
self._get_double_walk(),
*self._get_diagonal_walks(),
)
for target in targets:
if self.is_possible_target(board, target):
yield target

def __str__(self):
if self.color == Color.WHITE:
return '♙'
return '♟'

class Bishop(WalksDiagonallyMixin, WalksMultipleStepsMixin, Piece):
def __str__(self):
if self.color == Color.WHITE:
return '♗'
return '♝'

class Rook(WalksStraightMixin, WalksMultipleStepsMixin, Piece):
def __str__(self):
if self.color == Color.WHITE:
return '♖'
return '♜'

class Queen(
WalksStraightMixin, WalksDiagonallyMixin, WalksMultipleStepsMixin, Piece,
):
def __str__(self):
if self.color == Color.WHITE:
return '♕'
return '♛'

class Knight(Piece):
def __init__(self, **kwargs):
super().__init__(**kwargs)
self.directions = [
(-2, 1), (-1, 2), (1, 2), (2, 1),  # Upper part
(-2, -1), (-1, -2), (1, -2), (2, -1),  # Lower part
]

def get_valid_moves(self, board):
for row_change, col_change in self.directions:
row, col = self.get_position()
target = (row + row_change, col + col_change)
if self.is_possible_target(board, target):
yield target

def __str__(self):
if self.color == Color.WHITE:
return '♘'
return '♞'

class King(WalksStraightMixin, WalksDiagonallyMixin, Piece):
def _get_threatened_squares(self, board):
enemy = Color.enemy_of(self.color)
enemy_moves = list(board.get_possible_moves(enemy, with_king=False))
enemy_pieces = board.get_pieces(color=enemy)
king = next(p for p in enemy_pieces if isinstance(p, King))
for move in king.get_squares_threatens(board):
yield move
for move in enemy_moves:
yield move

def is_possible_target(self, board, target):
is_regular_valid = super().is_possible_target(board, target)
threatened_squares = self._get_threatened_squares(board)
return is_regular_valid and target not in threatened_squares

def get_valid_moves(self, board):
if self.is_possible_target(board, target):
yield target

def get_squares_threatens(self, board):
for direction in self.directions:
row, col = self.get_position()
row = row + direction[0]
col = col + direction[1]
if board.is_valid_square((row, col)):
yield (row, col)

def __str__(self):
if self.color == Color.WHITE:
return '♔'
return '♚'


Things I know I can improve, but I leave as-is because of my current student's knowledge:

1. I can use yield from instead of for x in y: yield x.
2. Color can inherit enum.Enum and use enum.auto() for the class variables.
3. I can raise exceptions instead of returning True or False.
• Regarding your last point, Returning True or False is likely better than raising errors anyways – user Jul 9 at 12:45

I would include a comment at the top of the file indicating the version of the relevant software you've used. A quick comment stating "Tested with Python 3.6 (installed through Anaconda)" or something to that effect is nice to make sure everyone is on the same page.

Since this is intended as teaching example, I will focus on minimizing the current code. I think that it is a reasonable assumption that more code gives more room for potential confusion.

class Board:
BOARD_SIZE = (8, 8)


Will you ever have a non-square board? Can this be a simple int? Changing this reduces the overall amount of code by a non-trivially amount.

    def get_square(self, row, col):
if not self.is_valid_square((row, col)):
return None
return self.board[row][col]

def set_square(self, row, col, piece):
self.board[row][col] = piece


Getters and setters are rare in Python, and since the board is public facing (it isn't prefixed with an underscore like later functions are), the setter doesn't really add much to the code. The getter smells a little bit, since a getter returning None is unexpected, and none of the provided code that uses the getter checks for None. I would remove both.

    def is_valid_square(self, square):
return (
square[0] in range(self.BOARD_SIZE[0])
and square[1] in range(self.BOARD_SIZE[1])
)


This function is not pleasant to debug if it is used incorrectly. An example of this is the error given if the input parameter 'square' is empty.

>>> board.is_valid_square([])
Traceback (most recent call last):
...
square[0] in range(self.BOARD_SIZE[0])
IndexError: list index out of range


Which list is indexed out of range? There are two index operations on the same line. There are also two different uses of the word range, each with different meanings. That could be confusing to a beginner.

Strictly speaking, the parameter square can be any size, but we expect it to be two elements big. I would make this assumption explicit with code through either an unpacking, an assert, or by changing the function signature.

def is_valid_square(self, row, col):
return row in range(self.BOARD_SIZE) and col in range(self.BOARD_SIZE)


def _generate_first_row(self, color):
row_by_color = {Color.BLACK: 0, Color.WHITE: self.BOARD_SIZE[0] - 1}
row = row_by_color[color]

order = (Rook, Knight, Bishop, Queen, King, Bishop, Knight, Rook)
params = {'color': color, 'row': row}
return [order[i](col=i, **params) for i in range(self.BOARD_SIZE[0])]


As a small thing, I would change the name to _generate_back_row. I think that is a slightly more clear name. A quick wikipedia search tells me that the exact term to use would be first-rank or back-rank, but that might might not be well enough known.

This function has a lot going on in it. I think this could be simplified a little, taking advantage of the fact there are only two colours. The dictionary lookup and expanding kwargs from a dictionary are overkill (but are both great things to teach, I would leave them in _generate_pawn). The code could look something like

def _generate_back_row(self, color):
row = 0 if color == Color.BLACK else self.BOARD_SIZE - 1

order = (Rook, Knight, Bishop, Queen, King, Bishop, Knight, Rook)
return [
order[i](col=i, row=row, color=color)
for i in range(self.BOARD_SIZE[0])
]


def get_pieces(self, color=None):
for row in self.board:
for col in row:
if col is not None and (color is None or col.color == color):
yield col


I think the variable col should be named square. What does color=None mean? Get both colours pieces? The feature isn't used anywhere in the code. I think this function should be made simpler, removing the default parameter. I think it would be more reasonable for the code to look like

def get_color_pieces(self, color):
for row in self.board:
for square in row:
if square is not None and square.color == color:
yield square


def get_possible_moves(self, color, with_king=False):
"""Return all player's possible moves."""


The comment is a little confusing. Which player are we talking about? What does with_king mean? I would have expected all possible moves to include those of the king by default. I would suggest something like below, which flips the default, including the possible king moves, but highlighting that the function can optionally not include them.

def get_possible_moves(self, color, exclude_king=False):
"""Return all possible moves using the pieces of the specified color."""


def is_possible_target(self, board, target):
is_target_valid = board.is_valid_square(target)
is_empty_square = board.is_empty_square(target)
is_hitting_enemy = self.is_enemy(board.get_square(*target))
return is_target_valid and (is_empty_square or is_hitting_enemy)


This is a good function. The names of the functions it calls make the logic clear and easy to follow. I would consider changing the definition to return is_target_valid and not is_hitting_self, since that would be less work for the computer, but overall this looks really good.

def is_enemy(self, piece):
if piece is None:
return False
return piece.color == Color.enemy_of(self.color)


This could be slightly more obvious by ending with return piece.color != self.color.

def get_valid_moves(self, board):
for row_change, col_change in self.directions:
steps = 1
stop_searching_in_this_direction = False
while not stop_searching_in_this_direction:
new_row = self.row + row_change * steps
new_col = self.col + col_change * steps
target = (new_row, new_col)
is_valid_target = self.is_possible_target(board, target)
if is_valid_target:
yield target
steps = steps + 1
is_hit_enemy = self.is_enemy(board.get_square(*target))
if not is_valid_target or (is_valid_target and is_hit_enemy):
stop_searching_in_this_direction = True


I would make some small changes to the logic of this function. It has quite a bit of complexity (3 indents, a yield, and an if statement that directly affects the next if statement), so giving it more space whitespace, and inverting some of the booleans might make it a little cleaner and more importantly, easier to parse.

The first thing to change is to move the inner logic to its own function. This has two benefits, it makes the code a little easier to parse, and it allows the inner logic to stop whenever it needs to, rather than tracking the loop condition explicitly.

def get_valid_moves(self, board):
for row_change, col_change in self.directions:
for move in moves_in_a_direction(self, row_change, col_change):
yield move

def moves_in_a_direction(self, row_change, col_change):
steps = 1
stop_searching_in_this_direction = False
while not stop_searching_in_this_direction:
new_row = self.row + row_change * steps
new_col = self.col + col_change * steps
target = (new_row, new_col)
is_valid_target = self.is_possible_target(board, target)
if is_valid_target:
yield target
steps = steps + 1
is_hit_enemy = self.is_enemy(board.get_square(*target))
if not is_valid_target or (is_valid_target and is_hit_enemy):
stop_searching_in_this_direction = True


is_hit_enemy is only set in the first if statement, it doesn't even exist before then. I would try and keep the logic to the one place (and change the name to has_hit_enemy, as that would be more accurate). To do this, invert the condition to make it a guard clause

if not is_valid_target:
return

yield target
steps += 1
has_hit_enemy = ...
...


This facilitates the removal of stop_searching_in_this_direction, as it was only used to stop the loop. Since we can return, it becomes unnecessary.

def moves_in_a_direction(self, row_change, col_change):
steps = 1
while True:
new_row = self.row + row_change * steps
new_col = self.col + col_change * steps
target = (new_row, new_col)
is_valid_target = self.is_possible_target(board, target)
if not is_valid_target:
return

yield target
steps = steps + 1

has_hit_enemy = self.is_enemy(board.get_square(*target))
if has_hit_enemy:
return


def _get_regular_walk(self):
src_row, src_col = self.get_position()
return (src_row + self.forward, src_col)


This looks ok, but src doesn't really mean anything here. I'd say drop it

def _get_regular_walk(self):
row, col = self.get_position()
return row + self.forward, col


In fact, since each piece knows its own row and column, why do we need self.get_position() anyway? It might be a candidate for deletion.

def _get_regular_walk(self):
return self.row + self.forward, self.col


def is_possible_target(self, board, target):
is_valid_move = board.is_valid_square(target)
is_step_forward = (
board.is_empty_square(target)
and target == self._get_regular_walk()
)
is_valid_double_step_forward = (
board.is_empty_square(target)
and not self.moved
and target == self._get_double_walk()
and self.is_possible_target(board, self._get_regular_walk())
)
is_hitting_enemy = (
self.is_enemy(board.get_square(*target))
and target in self._get_diagonal_walks()
)
return is_valid_move and (
is_step_forward or is_valid_double_step_forward or is_hitting_enemy
)


The logic looks good, but it is hard to find it in amongst the code. The more I see is_valid_square, the less I like the name. Consider other names that let you know what the function checks for, such as is_within_bounds or is_inside. I have also noticed that every function which returns a boolean has been prefixed with is_, to an almost pathological degree. There are other prefixes which would be much better suited, like has, can, will, or simply leaving out the prefix. With a guard clause, and changing the prefixes to make more sense, the code might look like

def is_possible_target(self, board, target):
is_valid_move = board.is_valid_square(target)
if not is_valid_move:
return False

can_step_forward = (
board.is_empty_square(target)
and target == self._get_regular_walk()
)

can_double_step_forward = (
can_step_forward and
not self.moved and
board.is_empty_square(target) and
target == self._get_double_walk()
)

can_capture = (
self.is_enemy(board.get_square(*target))
and target in self._get_diagonal_walks()
)

return can_step_forward or can_double_step_forward or can_capture


class King(WalksStraightMixin, WalksDiagonallyMixin, Piece):
def _get_threatened_squares(self, board):
enemy = Color.enemy_of(self.color)
enemy_moves = list(board.get_possible_moves(enemy, with_king=False))
enemy_pieces = board.get_pieces(color=enemy)
king = next(p for p in enemy_pieces if isinstance(p, King))
for move in king.get_squares_threatens(board):
yield move
for move in enemy_moves:
yield move


This is okay, but not as clear as it could be. Rearranging the lines and renaming king to enemy king improves the code.

class King(WalksStraightMixin, WalksDiagonallyMixin, Piece):
def _get_threatened_squares(self, board):
enemy = Color.enemy_of(self.color)
enemy_moves = list(board.get_possible_moves(enemy, exclude_king=True))
for move in enemy_moves:
yield move

enemy_pieces = board.get_pieces(color=enemy)
enemy_king = next(p for p in enemy_pieces if isinstance(p, King))
for move in enemy_king.get_squares_threatens(board):
yield move


But this brings up the question of "Why is the enemy king treated differently?" Surely it is just another enemy piece that has a set of possible moves, each of which threatens this king? If there is something of note here, a comment explaining it would be helpful.

• You are awesome. Thank you for the non-trivial amount of effort and the good spirit. I'm sure I'll take many of your advice. – Infinity Jul 6 at 1:50
• Will you ever have a non-square board? Will US zip codes ever be more than 5 digits? Will there ever be a need to specify century in dates? Will a person or place name ever be more than arbitrary number here characters? Will the customer ever want empty number fields to be null instead of zero? Will a social security number ever be legitimately reused? – radarbob Jul 10 at 17:54
• @radarbob Zipcodes can be nine digits long, called ZIP+4's, used when specifying a street address or Post Office Box. – Linny Jul 11 at 20:59

Since @spyr03's extensive (and awesome) answer did not include it, here are some small comments.

You want this to be an example for your students of how code should look like. You should include a docstring with every class, method and function to detail what it does and what it's arguments and return value are. Although your code is rather self-documenting, this sets the precedent for when they write their own code. If you consistently do it (and require it of them), some may learn to do it.

Don't forget to teach them about writing (good) tests either, at least eventually. Especially the pieces would be a good easy candidate for tests. They have complex non-trivial behavior which you could mess up when changing something, so having full test coverage on them would be very helpful.

On a practical note, I was slightly surprised when I came to the Pawn class. First, you define these nice mixins for the movement types. But then, the Pawn class does not to use any of them! I understand that the pawn is probably the first piece you want to define, and also that it is a bit hard to use the mixins in this case, but I would consider if it wouldn't be better to start with a piece that actually uses one of the mixins. Or define them later, when you actually need them.

When I think back to my student days, the most crucial point for me to understand code was always the entry point. In my experience, it takes a lot of experience to understand a code concept as a whole. The untrained is used to step by step thinking and evaluation through step by step progression. I would not have understood that code because it describes the game and does not PLAY the game. I understood, that the code is not meant to play. But a clearly marked start() function initializing the board and doing some sample moves so the student can see and visualize how the code come together and what it can actually do, would help a lot. At least it would have helped me.

• "Yes, and..." If you take that code that "initializes the board and does some sample moves" and then add a few asserts at the end — ta-da, you have your first unit test! Unit tests can help the reader understand the behavior of the code, and its intended usage, and serve as, well, tests, all at the same time. Highly recommended. – Quuxplusone Jul 7 at 3:55

One thing I don't see mentioned in spyr03's excellent review: I think it's unnecessarily inconsistent (and thus confusing) for you to use mixin classes to implement 100% of the get_valid_moves routine for Rook, Bishop, and Queen, but then reuse only half of that code for King (and open-code the other half). If you're going to write

class Queen(
WalksStraightMixin, WalksDiagonallyMixin, WalksMultipleStepsMixin, Piece,
):                                            #######################
def __str__(self):
if self.color == Color.WHITE:
return '♕'
return '♛'


then you should also write

class King(
WalksStraightMixin, WalksDiagonallyMixin, WalksSingleStepMixin, Piece,
):                                            ####################


It's gratuitously confusing to have WalksStraightMixin and WalksDiagonallyMixin set values into self.directions that are then read by King itself. This is a tightly coupled dependency between the mixins and the implementation of King; consider how many places in the code you'd have to change if you wanted to rename directions to possibleDirections, or something like that.

In real life, btw, I would consider your mixin idea to be much too complicated. We could "keep it simple" by manually implementing get_valid_moves for each class individually:

class Piece:
straight_directions = [...]
diagonal_directions = [...]
all_directions = straight_directions + diagonal_directions
def get_single_step_moves_impl(directions): ...
def get_multistep_moves_impl(directions): ...

class Rook(Piece):
def get_valid_moves(self):
return self.get_multistep_moves_impl(Piece.straight_directions)

class Queen(Piece):
def get_valid_moves(self):
return self.get_multistep_moves_impl(Piece.all_directions)

class King(Piece):
def get_valid_moves(self):
return self.get_single_step_moves_impl(Piece.all_directions)


Here, rather than inheriting from mixins — that might conceivably affect the behavior of the whole class — we limit our "different" effects to the smallest possible scope. The difference between Queen's use of get_multistep_moves_impl and King's use of get_single_step_moves_impl is clearly restricted to get_valid_moves only; Queen and King clearly don't differ in anything other than the behavior of get_valid_moves (not as presented above, anyway). This limitation-of-possible-effects makes it easier for the reader to reason about the code.