patternpythonMinor
Quarto game in Python revisited
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
```
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
I like Google-style docstrings for human-readable docstrings that e.g. Sphinx can also understand.
With the
Alternatively, I would be tempted to make that a
Note that you don't need the class attributes. This becomes more important when you get to the
I'd also change the way you output the board; instead of
An important note is that:
seems like a lot of complexity around what you actually mean:
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
You could also make
"""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 aboveYou 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 aboveContext
StackExchange Code Review Q#148163, answer score: 4
Revisions (0)
No revisions yet.