4
\$\begingroup\$

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[coord]
        except KeyError:
            raise ValueError("coord does not exist")

    def filled(self):
        """
        Return amount of spaces filled.
        """
        f = 0

        for v in self.board.itervalues():
            if v != ' ':
                f += 1

        return f

    def has_row(self):
        """
        Return a list of players who have a row of three.
        """
        across = [[i for i in itertools.product([0,1,2], [n])] for n in xrange(3)]
        down = [[(b, a) for a, b in i] for i in across]
        diag = [
            [(0,0), (1,1), (2,2)],
            [(0,2), (1,1), (2,0)],
        ]

        results = []

        for row in itertools.chain(across, down, diag):
            values = [self.board[c] for c in row]

            if values == ['x', 'x', 'x']:
                results.append('x')
            if values == ['o', 'o', 'o']:
                results.append('o')

        return results
\$\endgroup\$
3
  • \$\begingroup\$ Assigning a name for the itertools like import itertools like it would help you reduce the line size and multiple typing. \$\endgroup\$
    – Dawny33
    Feb 8 '16 at 9:58
  • \$\begingroup\$ @Dawny33. Typo? import itertools as it \$\endgroup\$
    – boardrider
    Feb 9 '16 at 22:49
  • \$\begingroup\$ @boardrider Yeah. Thank you! Too late to edit now :) \$\endgroup\$
    – Dawny33
    Feb 10 '16 at 2:13
3
\$\begingroup\$

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.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks for the feedback. I'm not sure why you suggest using range instead of xrange? There is little difference in performance here I know, but xrange seems more suitable as I'm not storing list that range would produce. And why use map here? The list comp is more readable to my mind, but perhaps that is because I haven't encountered map much. \$\endgroup\$
    – rosofo
    Feb 8 '16 at 16:37
  • \$\begingroup\$ @Rickover: the map I'm undecided as to which is better for this. Both are clear. Regarding the range: the performance/memory difference in this case are probably negligable. But xrange is gone in Python3, so porting becomes less of a 'hazzle'. Hence I prefer range when storing is not an issue. \$\endgroup\$ Feb 8 '16 at 17:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.