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()

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>)