Object-oriented Tic-Tac-Toe game

Question

I implemented an object-oriented version of a Tic-Tac-Toe game in Python 3.

I'd like to have feedback on general code style and possible optimizations.

#! /usr/bin/env python3
#
#    tictactoe - A simple tic-tac-toe game
#
#    (C) 2017 - Richard Neumann <mail at richard dash neumann dot de>
#
#    This program is free software: you can redistribute it and/or modify
#    it under the terms of the GNU General Public License as published by
#    the Free Software Foundation, either version 3 of the License, or
#    (at your option) any later version.
#
#    This program is distributed in the hope that it will be useful,
#    but WITHOUT ANY WARRANTY; without even the implied warranty of
#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
#    GNU General Public License for more details.
#
#    You should have received a copy of the GNU General Public License
#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
#
"""A simple Tic-Tac-Toe game"""

from itertools import chain


class InvalidInput(Exception):
    """Indicates an invalid user input"""

    def __init__(self, message):
        """Sets the message to the user"""
        super().__init__(message)
        self.message = message


class FieldIsOccupied(Exception):
    """Indicates that the selected field is occupied"""

    pass


class GameEnd(Exception):
    """Indicates the end of the game"""

    def __init__(self, winner):
        """Sets the respective winner"""
        super().__init__(winner)
        self.winner = winner


class Player():
    """Player and field values"""

    CROSS = 'x'
    CIRCLE = 'o'
    NONE = ' '


class TicTacToe():
    """The game class"""

    def __init__(self):
        """Initializes field and last player"""
        self.field = [[Player.NONE] * 3, [Player.NONE] * 3, [Player.NONE] * 3]
        self.last_player = Player.NONE

    def __str__(self):
        """Converts the game field into a string"""
        return '{}║{}║{}\n═╬═╬═\n{}║{}║{}\n═╬═╬═\n{}║{}║{}'.format(
            *chain(*self.field))

    @property
    def win_patterns(self):
        """Yields patterns significant for winning"""
        # Rows
        yield self.field[0]
        yield self.field[1]
        yield self.field[2]
        # Columns
        yield (self.field[0][0], self.field[1][0], self.field[2][0])
        yield (self.field[0][1], self.field[1][1], self.field[2][1])
        yield (self.field[0][2], self.field[1][2], self.field[2][2])
        # Diagonals
        yield (self.field[0][0], self.field[1][1], self.field[2][2])
        yield (self.field[0][2], self.field[1][1], self.field[2][0])

    @property
    def next_player(self):
        """Returns the next player"""
        if self.last_player is Player.CROSS:
            return Player.CIRCLE

        return Player.CROSS

    def check_winner(self):
        """Check if the game has ended"""
        draw = True

        for fields in self.win_patterns:
            if fields[0] in (Player.CROSS, Player.CIRCLE):
                if all(fields[0] == field for field in fields[1:]):
                    raise GameEnd(fields[0])
            elif any(field is Player.NONE for field in fields):
                draw = False

        if draw:
            raise GameEnd(Player.NONE)

    def make_turn(self, player, column, row):
        """Makes a turn"""
        if self.field[row][column] is Player.NONE:
            self.last_player = self.field[row][column] = player
        else:
            raise FieldIsOccupied()

    @property
    def player_input(self):
        """Reads input from player"""
        coords = input('Turn for {}: '.format(self.next_player))

        try:
            column, row = coords.split()
        except ValueError:
            raise InvalidInput('Please enter: "<column> <row>"')
        else:
            try:
                column, row = int(column), int(row)
            except ValueError:
                raise InvalidInput('Coordinates must be integers.')
            else:
                if all(0 <= i <= 2 for i in (column, row)):
                    return column, row

                raise InvalidInput('Coordinates must be 0 <= i <=2.')

    def start(self):
        """Starts the game"""
        while True:
            print(self)

            try:
                self.check_winner()
            except GameEnd as game_end:
                if game_end.winner is Player.CROSS:
                    print('Cross wins.')
                elif game_end.winner is Player.CIRCLE:
                    print('Circle wins.')
                else:
                    print('The game ended in a draw.')

                break

            try:
                column, row = self.player_input
            except KeyboardInterrupt:
                print('\nGame aborted.')
                return False
            except InvalidInput as invalid_input:
                print(invalid_input.message)
            else:
                try:
                    self.make_turn(self.next_player, column, row)
                except FieldIsOccupied:
                    print('The selected field is occupied.')

        try:
            input('Press enter to exit...')
        except KeyboardInterrupt:
            print()


def main():
    """Runs a new game"""

    game = TicTacToe()
    game.start()


if __name__ == '__main__':
    main()

Answer 1

Great job there - easy to understand class with a nice separation logic around the methods - really like that you've used __str__ to have a string representation of the game board.

Some minor improvements:

• you can remove extra () from the class definitions:

  class TicTacToe:
  

• end your docstrings with a dot (see PEP 257 -- Docstring Conventions)

• you can use the short if/else form in your next_player method:

  return Player.CIRCLE if self.last_player is Player.CROSS else Player.CROSS
  

• Player can be an Enum class. And, think about a better name for it - it feels more like a Field than Player since you enumerate possible field values there

Note that it is not straightforward to scale the board to a bigger size - not sure how big of a problem this is, but think about adding more generalizations to the class.

Answer 2

it's looking very good! I had a few things to talk about.

Don't use is for string comparison, use ==. If the test is for checking equality, using is will give inconsistent results.

See this question https://stackoverflow.com/questions/1504717/why-does-comparing-strings-in-python-using-either-or-is-sometimes-produce

Exception Handling, I think overall the exception handling is very good, but I think you may have gone a little overboard with regards to your GameEnd Exception class. I like FieldIsOccupied Exception, though. An exception is supposed to be raised when an Exceptional thing happens in your program, I would say that someone trying to fill an already filled slot is that. But the end of the game is very much a core part of the flow of your game!

To me, something like this makes more sense.

if self.has_winner():
    winner = self.winner
    ...
else:
    ...

Using exceptions as control flow is considered an anit-pattern.

On top of this, I would also say that a ValueError could be used in place of an InvalidInput