HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Quarto game in Python revisited

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
revisitedquartogamepython

Problem

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

Solution

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

Code Snippets

"""All on one line."""
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',
        ])
def __init__(self, tall, black, circle, solid_top):
    ...

@classmethod
def from_int(cls, attributes):
    return cls(attributes & 0b0001, ...)
self.pieces = [Piece(int("".join(seq),2)) for seq in itertools.product("01", repeat=4)]
self.pieces = [Piece(i) for i in range(16)]  # or Piece.from_int(i) as above

Context

StackExchange Code Review Q#148163, answer score: 4

Revisions (0)

No revisions yet.