Quarto game in Python revisited

Question

A while ago, I posted about advice for refactoring a game of Quarto I had coded before. I got some good advice, and did some revisions in lieu of this.

import itertools
# Enums for quarto.
DRAW = "D"
WIN = "W"
UNDECIDED = "U"
LOSE = "L"
TIE = "T"
class Piece():
 """ 
 Piece class: Each piece has a name (string with its four attributes), an abbreviation 
 (string with the first letter of each attribute), and a list of its attributes.
 """
 attributes = None
 full_name = ""
 abbreviation = ""
 def __init__(self, attributes):
 """ 
 Constructor for our piece class: Takes in a list of attributes and builds the full_name
 and abbreviation strings from it.
 """
 self.attributes = attributes
 if attributes & 0b0001:
 self.full_name += "Tall"
 self.abbreviation += "T"
 else:
 self.full_name += "Short"
 self.abbreviation += "S"
 if attributes & 0b0010:
 self.full_name += " black"
 self.abbreviation += "B"
 else:
 self.full_name += " white"
 self.abbreviation += "W"
 if attributes & 0b0100:
 self.full_name += " circle"
 self.abbreviation += "C"
 else:
 self.full_name += " square"
 self.abbreviation += "Q"
 if attributes & 0b1000:
 self.full_name += " solid-top"
 self.abbreviation += "D"
 else:
 self.full_name += " hollow-top"
 self.abbreviation += "H"
class Board():
 """
 Board class: Each board is a 4x4 2D array with a list of 16 pieces.
 """
 pieces = []
 cols_count = 4
 rows_count = 4
 board = None
 def __init__(self):
 """
 Constructor for our board class. 
 Generates binary representations for all the pieces with the following 
 meanings:
 #short: 0, tall: 1
 #black: 0, white: 1
 #circle: 0, square: 1
 #solid: 0, hollow: 1
 Appends all the pieces of quarto to the array of pieces.
 Initializes the board, implemented by a 4x4 2D array.
 """
 self.board = [[None for x in range(self.cols_count)] for y in
 range(self.rows_count)]
 #short: 0, tall: 1
 #black: 0, white: 1
 #circle: 0, square: 1
 #solid: 0, hollow: 1
 self.pieces = [Piece(int("".join(seq),2)) for seq in itertools.product("01", repeat=4)]
 @property
 def pieces_names(self):
 """
 Returns the names of all the pieces in our pieces array.
 """
 return ["(" + str(i) + "): " + self.pieces[i].full_name
 for i in range(0, len(self.pieces))]
 @property
 def rows(self):
 """
 Returns the rows of the board.
 """
 return self.board
 @property
 def cols(self):
 """
 Returns the columns of the board.
 """
 return zip(* self.board)
 def shared_attributes(self, lst):
 """
 Returns a list of shared attributes among pieces.
 """
 attributes_list = [piece.attributes for piece in lst
 if piece] #nested list of all attributes
 if len(attributes_list) != self.rows_count:
 return 0
 win = 0b1111
 win2 = 0b1111
 for attr in attributes_list:
 win = win & attr
 win2 = win2 & ~attr
 return win or win2
 def check_win_horizontal(self):
 """
 Returns a boolean indicating whether or not a horizontal win has occurred.
 """
 return any(self.shared_attributes(row) for row in self.rows)
 def check_win_vertical(self):
 """
 Returns a boolean indicating whether or not a vertical win has occurred.
 """
 return any(self.shared_attributes(col) for col in self.cols)
 def check_win_diagonal(self):
 """
 Returns a boolean indicating whether or not a diagonal win has occurred.
 """
 ltr_diag = []
 rtl_diag = []
 i, j = 0, 3
 for row in self.board:
 ltr_diag += [row[i]]
 rtl_diag += [row[j]]
 i += 1
 j -= 1
 if (self.shared_attributes(ltr_diag) or
 self.shared_attributes(rtl_diag)):
 return True
 return False
 def player(self):
 """
 Returns whose turn it is.
 """
 if len(self.pieces)%2 == 0:
 return 1
 else:
 return 2
 def other_player(self):
 """ 
 Returns the other person's turn.
 """
 if len(self.pieces)%2 == 0:
 return 2
 else:
 return 1
 def print_board(self):
 """
 Prints the board.
 """
 for row in self.board:
 pr = []
 for piece in row:
 if piece:
 pr.append(piece.abbreviation)
 else:
 pr.append(None)
 print(pr)
 def place_piece(self, piece, row, col):
 """
 Places the piece in the specified location and removes the piece from the list of available
 pieces. 
 """
 if not self.board[row][col] and piece in self.pieces:
 self.board[row][col] = piece
 # del self.pieces[piece]
 self.pieces.remove(piece)
 return True
 else:
 return False
quarto_board = None
def initial_position():
 """
 Initializes the board, which is a 4x4 2D array.
 """
 return Board()
def primitive(state):
 """
 Returns whether or not the current game is a TIE, WIN, or UNDECIDED.
 This function takes in a state, which is an instance of Board.
 """
 if (state.check_win_horizontal() or state.check_win_vertical() or
 state.check_win_diagonal()):
 return WIN
 # no more pieces
 if len(state.pieces) == 0:
 return TIE
 return UNDECIDED
def main():
 """
 The main function that keeps the game running.
 This is essentially an interface between our quarto game
 and the player.
 """
 print("Starting game of Quarto...")
 print()
 board = initial_position()
 while True:
 print("------------")
 print("Player", board.player(), "'s turn: ")
 print("------------")
 print("Current state of board: ")
 board.print_board()
 print()
 print("Available pieces: ")
 print(board.pieces_names)
 print()
 while True:
 try: 
 piece_index = input("Player " +str(board.other_player()) +
 " please pick a piece to give to Player "
 + str(board.player()) +
 " (index num): ")
 int_piece_index = int(piece_index)
 if int_piece_index >= len(board.pieces) or int_piece_index < 0:
 raise ValueError
 break
 except ValueError:
 print("Index out of bounds. Choose a piece index between 0 and " + str(len(board.pieces) - 1) + ".")
 p = board.pieces[int(piece_index)]
 while True: #checks if that position on the board is already full
 print("Player " + str(board.player()) + " choose where you want to place " + p.full_name + "...")
 try:
 while True: #checks for valid row index
 try: 
 r = int(input("Row: "))
 if r >= board.rows_count or r < 0:
 raise ValueError
 break
 except ValueError:
 print("Index out of bounds. Choose a row index between 0 and 3.")
 while True: #checks for valid column index
 try: 
 c = int(input("Col: "))
 if c >= board.cols_count or c < 0:
 raise ValueError
 break
 except ValueError:
 print("Index out of bounds. Choose a column index between 0 and 3.")
 success = board.place_piece(p, r, c)
 if not success:
 raise ValueError
 break
 except ValueError:
 print("That space on the board is already occupied. Choose a different row and column index.")
 if primitive(board) == WIN:
 board.print_board()
 print("Player", board.other_player(), "wins!")
 return
 else:
 print("SOLVE: ", primitive(board))
 print()
main()

Just as a second sanity check, I was wondering if there were any other revisions that would be good to implement? Thanks!

Answer 1

One tiny thing to start; your docstrings are not compliant with PEP-257, which lays out the conventions. For example, a short docstring like "Returns the rows of the board." should be

"""All on one line."""

I like Google-style docstrings for human-readable docstrings that e.g. Sphinx can also understand.

---

With the Piece, why not deconstruct that information a bit more? Each attribute you need can then be built up from the basics.

class Piece():
 """..."""
 def __init__(self, attributes):
 self.tall = attributes & 0b0001
 self.black = attributes & 0b0010
 self.circle = attributes & 0b0100
 self.solid_top = attributes & 0b1000
 @property
 def full_name(self):
 return ' '.join([
 'Tall' if self.tall else 'Short',
 'black' if self.black else 'white',
 'circle' if self.circle else 'square',
 'solid-top' if self.solid_top else 'hollow-top',
 ])
 @property
 def abbreviation(self):
 return ''.join([
 'T' if self.tall else 'S',
 'B' if self.black else 'W',
 'C' if self.circle else 'Q',
 'D' if self.solid_top else 'H',
 ])

Alternatively, I would be tempted to make that a from_int class method, so that the __init__ method takes the actual attributes of the instance:

def __init__(self, tall, black, circle, solid_top):
 ...
@classmethod
def from_int(cls, attributes):
 return cls(attributes & 0b0001, ...)

---

Note that you don't need the class attributes. This becomes more important when you get to the Board, where you have a mutable class attribute that you then treat as if it were an instance attribute. This would get you into trouble if you tried to create more than one board at a time, see e.g. How do I avoid having class data shared among instances?

I'd also change the way you output the board; instead of print_board use the __str__ magic method to return a string representation and simply print str(board) as necessary.

An important note is that:

self.pieces = [Piece(int("".join(seq),2)) for seq in itertools.product("01", repeat=4)]

seems like a lot of complexity around what you actually mean:

self.pieces = [Piece(i) for i in range(16)] # or Piece.from_int(i) as above

You don't need to build the binary string then convert it back into an integer.

---

There is an awful lot of complexity in your while loop in main. I would break some of that out into separate functions, such that you ended up with e.g.:

def main():
 """..."""
 print("Starting game of Quarto...\n")
 board = initial_position()
 while True:
 show_current_state(board)
 piece = select_piece(board)
 row, col = get_valid_location(board)
 board.place_piece(piece, row, col)
 state = primitive(board)
 if state == WIN:
 show_winner(board)
 else:
 print("SOLVE: ", state)
 print()

You could also make primitive a method on the Board; state = board.primitive(). initial_position seems a little bit pointless, adding a layer of indirection that doesn't actually supply any extra information. The code would be easier to read, in my opinion, as simply:

board = Board()

• Caridorc: abbreviation should call fullname to avoid repetition. Or a helper that returns a list.
• jonrsharpe: @Caridorc it's not quite so simple as it's not a straightforward abbreviation.
• Caridorc: I understand now, you meant that you need to avoid repetition of letters.
• jonrsharpe: @Caridorc yes, if it was just the first letter you could reuse part of it, but the Q and D complicate things

Answer 2

# Enums for quarto.
DRAW = "D"
WIN = "W"
UNDECIDED = "U"
LOSE = "L"
TIE = "T"

This calls for a real enum. Also you seems to only use 3 of them. Remove DRAW and LOSE if they are useless:

import enum
class GameState(enum.Enum):
 UNDECIDED = 1
 TIE = 2
 WIN = 3

---

You also use a lot the pattern

while True:
 try:
 ...
 if not <condition>:
 raise ValueError
 break
 except ValueError:
 print(<message>)

First off, putting the code that has nothing to do with the control flow (... here) under the try is a bad idea since a ValueError from there, if any, would be understood as being part of the failure of the condition.

Second, why on earth would you complicate things by using exceptions as control flow when you already use an if to check if things went right or not:

while True:
 ...
 if <condition>:
 break
 print(<message>)