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

Tic-Tac-Toe in Python 2.x

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

Problem

This is a class called Board which handles the tracking and checking of the state of the game. self.board is a dict of coordinate: state pairs, for e.g. (0,1): 'x'.

I could have hardcoded the checking lists in but decided to be adventurous and learn a bit of itertools.

The complete program is about 130 lines of code and I somehow managed to cram an ASCII board using a coordinate printer thingamajig in there (not included in this sample).

This is the code of a Pythoner 4 months into learning his first language. I seem to be very concerned about documentation, though I may just be in love with doctests.

``
class Board(object):
"""
A tictactoe board using coordinates.

Coordinates start at (0,0) in the top left.

>>> myboard = Board()

>>> myboard.set((0,1), 'x')
>>> myboard.set((0,1), 'foo')
Traceback (most recent call last):
...
ValueError: state must be one of 'x', 'o', or ' '
>>> myboard.set((7,1), 'o')
Traceback (most recent call last):
...
ValueError: coord does not exist

>>> myboard.get((0,1))
'x'
>>> myboard.get('banana')
Traceback (most recent call last):
...
ValueError: coord does not exist

>>> myboard.set((0,0), 'x')
>>> myboard.has_row()
[]
>>> myboard.set((0,2), 'x')
>>> myboard.has_row()
['x']
"""

def __init__(self):
self.board = {i: " " for i in itertools.product((0,1,2), repeat=2)}

def set(self, coord, state):
"""
Set a space on the board to
state.

For e.g.: set((0,1), 'x')
"""
if state not in ('x', 'o', ' '):
raise ValueError("state must be one of 'x', 'o', or ' '")
elif coord not in self.board.keys():
raise ValueError("coord does not exist")
else:
self.board[coord] = state

def get(self, coord):
"""
Get the state of the space at
coord`.
"""
try:
return self.board[co

Solution

First of all: great work on the doctests.

Your spacing could be improved, you might want to run a tool like pep8 over your code and fix the issues it reports: (0, 1, 2) instead of (0,1,2) and such.

Your set method is quite defensive, which is a good thing. However, do you really want to allow setting a field to ' ' when it already contains a 'x' or 'o'? Likewise: Do you want to allow placing something on a field that already contains an 'x' or an 'o'? Either way is fine, depending on if you want the board to know about the rules of the game or not.

One thing I would recommend changing is replacing the elif. Prefer

elif coord not in self.board:


over

elif coord not in self.board.keys():


It's a bit more idiomatic.

The method filled could also be a bit more simple. You're looping over all the values. Using a list comprehension, you could write

return len([x for x in self.board.itervalues() if x != ' '])


There are probably many more ways to rewrite it, though.

Your definition of across:

across = [[i for i in itertools.product([0,1,2], [n])] for n in xrange(3)]


That looks quite complicated to me.

[i for i in itertools.product([0, 1, 2], [n])]


can be replaced by

list(itertools.product([0, 1, 2], [n]))


or even better:

[(i, n) for i in (0, 1, 2)]


This gives

across = [[(i, n) for i in (0, 1, 2)] for n in xrange(3)]


It seems a bit silly to use both (0, 1, 2) and xrange(3). I'd suggest replacing both by range(3) or (0, 1, 2). Using xrange is a bit superfluous for ranges of length 3.

across = [[(i, n) for i in range(3)] for n in xrange(3)]


Ideally, you'd want to just mirror the definition for down, instead of re-using across, as it more cleanly symbolises the symmetry

down = [[(n, i) for i in range(3)] for n in range(3)]


Beyond that, your rows is quite clean, except the definition of values could be replaced by

values = map(self.board.get, row)


But that doesn't matter that much.

Code Snippets

elif coord not in self.board:
elif coord not in self.board.keys():
return len([x for x in self.board.itervalues() if x != ' '])
across = [[i for i in itertools.product([0,1,2], [n])] for n in xrange(3)]
[i for i in itertools.product([0, 1, 2], [n])]

Context

StackExchange Code Review Q#119258, answer score: 3

Revisions (0)

No revisions yet.