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.

• Richard Neumann: The class Player was actually named FieldValue before I changed it. :D Good point with the if/else shorthand. I will definitely change this.
• Richard Neumann: With your proposed one-line if/else I'd exceed the maximum line length of 79 characters. Since breaking up the statement into multiple lines using paratheses or `\` would void its purpose, I'll keep it as it is for now.
• Solomon Ucko: You could use booleans for the players, and just use return not last_player to switch turns.
• Ashwini Chaudhary: Also I am not sure whether their usage of message in InvalidInput is just a habit from Python 2 or it was intentional. message attribute was removed in Python 3 and str(exc) is the recommended way.

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