trjhtr

This is a console based Tic Tac Toe Game I programmed to practice my python. It is object oriented with a few standalone functions for getting user input. The game also has a basic AI that is implemented in a recursive function. I would appreciate suggestions as to what to work on or improvements to the way the game functions.

main.py

from TicTacToe import TicTacToe


def main():
 game = TicTacToe()
 game.run()


if __name__ == "__main__":
 main()

TicTacToe.py

from Board import *
import Players


class TicTacToe:
 def __init__(self):
 print("Welcome to Tic Tac Toe!")
 self.board = None
 self.player1 = None
 self.player2 = None
 self.players = [None, None]

 self.new_board()
 self.new_players()

 def new_board(self) -> None:
 self.board = Board()

 def new_players(self) -> None:
 player1type, player2type = get_player_types()
 self.player1 = player1type(None)
 self.player2 = player2type(get_enemy(self.player1.mark))
 self.players = [self.player1, self.player2]

 def is_game_complete(self) -> bool:
 state = self.board.winner()
 if state is None:
 return False
 else:
 self.board.print()
 if state == Board.TIE:
 print("Tie!")
 else:
 for player in self.players:
 if player.mark == state:
 print(player.name + " has won!")
 return True

 def run(self) -> None:
 game_running = True
 while game_running:
 for player in self.players:
 self.board.print()
 print("It is " + player.name + "'s turn.")
 move = player.get_move(self.board)
 self.board.make_move(move, player.mark)
 print("" + player.name + " has chosen tile " + str(move + 1) + ". ")

 if self.is_game_complete():
 if prompt_play_again():
 self.new_board()
 else:
 game_running = False
 break # Breaks from for loop


def get_player_types() -> (object, object):
 players = get_player_number()
 if players == 0:
 return Players.Computer, Players.Computer
 if players == 1:
 return Players.Human, Players.Computer
 if players == 2:
 return Players.Human, Players.Human


def get_player_number() -> int:
 while True:
 print("Please enter number of Human Players (0-2).")
 try:
 players = int(input(">>> "))
 assert players in (0, 1, 2)
 return players
 except ValueError:
 print("tThat is not a valid number. Try again.")
 except AssertionError:
 print("tPlease enter a number 0 through 2.")


def prompt_play_again() -> bool:
 while True:
 print("Would you like to play again? (Y/N)")
 response = input(">>> ").upper()
 if response == "Y":
 return True
 elif response == "N":
 return False
 else:
 print("Invalid input. Please enter 'Y' or 'N'.")

Board.py

class Board:
 X_MARK = "X"
 O_MARK = "O"
 PLAYERS = (X_MARK, O_MARK)
 TIE = "T"
 BLANK = None

 winning_combos = (
 [0, 1, 2], [3, 4, 5], [6, 7, 8],
 [0, 3, 6], [1, 4, 7], [2, 5, 8],
 [0, 4, 8], [2, 4, 6])

 # unicode characters
 vrt_line = chr(9475)
 hrz_line = chr(9473)
 cross = chr(9547)

 def __init__(self):
 self.board = [Board.BLANK] * 9

 def __str__(self):
 s = u"n"
 s += " 1 | 2 | 3 n"
 s += "---+---+---n"
 s += " 4 | 5 | 6 n"
 s += "---+---+---n"
 s += " 7 | 8 | 9 n"

 s = s.replace("|", Board.vrt_line)
 s = s.replace('-', Board.hrz_line)
 s = s.replace('+', Board.cross)

 for tile in range(9):
 if self.get_tile(tile) is not None:
 s = s.replace(str(tile + 1), self.board[tile])
 return s

 def print(self):
 print(str(self))

 def get_tile(self, key):
 return self.board[key]

 def make_move(self, key, player):
 if self.board[key] is None:
 self.board[key] = player
 return True
 return False

 def clear_tile(self, key):
 self.board[key] = Board.BLANK

 def available_moves(self) -> list:
 return [key for key, value in enumerate(self.board) if value is None]

 def get_tiles(self, player) -> list:
 return [key for key, value in enumerate(self.board) if value == player]

 def winner(self):
 for player in Board.PLAYERS:
 positions = self.get_tiles(player)
 for combo in Board.winning_combos:
 win = True
 for pos in combo:
 if pos not in positions:
 win = False
 if win:
 return player

 if len(self.available_moves()) == 0:
 return Board.TIE
 return None


def get_enemy(player):
 if player == Board.X_MARK:
 return Board.O_MARK
 elif player == Board.O_MARK:
 return Board.X_MARK
 else:
 return None

Players.py

from Board import *
from Exceptions import *
import random


class Player:

 def __init__(self, mark=None):
 if mark is None:
 self.mark = random.choice(Board.PLAYERS)
 else:
 self.mark = mark

 def get_move(self, board):
 pass


class Human(Player):

 count = 0

 def __init__(self, mark=None):
 Human.count += 1
 self.id = "Player " + str(Human.count)

 self.name = self.get_name()
 if mark is None:
 mark = self.get_mark()

 super(Human, self).__init__(mark)

 def get_move(self, board):
 available_moves = board.available_moves()
 while True:
 try:
 print("nWhere would you like to place an '" + self.mark + "'")
 move = int(input(">>> ")) - 1

 if move < 0 or move >= 9:
 raise InvalidMove
 if move not in available_moves:
 raise UnavailableMove
 return move

 except InvalidMove:
 print("That is not a valid square.",
 "Please choose another.")

 except UnavailableMove:
 print("That square has already been taken.",
 "Please choose another.")

 except ValueError:
 print("Error converting input to a number.",
 "Please enter the number (1-9) of the square you wish to take.")

 def get_mark(self):
 print("Hello " + self.name + "! Would you like to be 'X' or 'O'?")
 while True:
 mark = input(">>> ").upper()
 if mark in (Board.X_MARK, Board.O_MARK):
 return mark
 else:
 print("Input unrecognized. Please enter 'X' or 'O'.")

 def get_name(self):
 print(self.id + ", what is your name? ")
 return input(">>> ")


class Computer(Player):

 count = 0

 def __init__(self, mark=None):
 Computer.count += 1
 self.id = "Computer " + str(Computer.count)
 self.name = self.id

 super(Computer, self).__init__(mark)

 def __del__(self):
 Computer.count -= 1

 def get_move(self, board):
 best_score = -2
 best_moves = 
 available_moves = board.available_moves()

 if len(available_moves) == 9:
 return 4
 for move in available_moves:
 board.make_move(move, self.mark)
 move_score = self.min_max(board, get_enemy(self.mark))
 board.clear_tile(move)

 if move_score > best_score:
 best_score = move_score
 best_moves = [move]
 elif move_score == best_score:
 best_moves.append(move)
 move = random.choice(best_moves)
 return move

 def min_max(self, board, mark):
 winner = board.winner()
 if winner == self.mark:
 return 1
 elif winner == Board.TIE:
 return 0
 elif winner == get_enemy(self.mark):
 return -1
 available_moves = board.available_moves()

 best_score = None
 for move in available_moves:
 board.make_move(move, mark)
 move_score = self.min_max(board, get_enemy(mark))
 board.clear_tile(move)

 if best_score is None:
 best_score = move_score

 if mark == self.mark:
 if move_score > best_score:
 best_score = move_score
 else:
 if move_score < best_score:
 best_score = move_score
 return best_score

Exceptions.py

class InvalidMove(ValueError):
 def __init__(self, *args):
 super(InvalidMove, self).__init__(*args)


class UnavailableMove(ValueError):
 def __init__(self, *args):
 super(UnavailableMove, self).__init__(*args)

Here is my list of thoughts (in random order). Since you don't specify any particular goal I am reviewing mainly for "relative beauty" of the code:

• you have self.player1, self.player2 and self.players and you only reference self.players. You can refactor to remove self.player1 and self.player2.


• personally, I would put the code in your main.py onto the bottom of TicTacToe.py and get rid of main(). You can still import from TicTacToe, because thats what the if __name__[...] guards against. It makes more sense to me to call python tictactoe.py to run it or, if you fancy, create an __init__.py and move the code from main.py there. That way you can call the folder to play or import the folder (making a few adjustments) as a library


• I would not do a play again within the TicTacToe class. Instead, deconstruct the entire class and build a new one. Rather you bin it all, build anew and be certain you don't miss any variables still floating around


• I would move new_board() and new_players() into __init__(). You only seem to use them once so there is no need for them to be functions.


• you could get rid of the entire TicTacToe.py and instead maintain 3 objects (2 players and the board) in main.py. You can add another class, Score or scoreboard, for tracking and reporting scores between rounds.


• I wouldn't hide the game loop inside a method of a class. It doesn't just do some small thing with the class. It modifies a lot of things in different places. It also is the central piece of code. I would move that into main.py.

Board.py

• 
  title is not a static member of the class; you change it dynamically. Make it a property of the object.


• you don't need a print function. If you want to print to stdout use the buildin print where appropriate print(board_object). This allows the caller to choose where to print it, e.g. my_fancy_custom_string_stream.write(board_object)
  


• you can implement getters and setters more beautifully using @property
  


• the board shouldn't care which player has won. It shouldn't even know about players. That should either be tictactoe.py or what ever scoring method you choose to apply on the current board state


• same for rule enforcement on moves; though one can argue here. Is that that we constrain the board to not fit multiple pieces in one place (in which case the board shouldn't care) or is it that multiple pieces simply don't fit (in which case we shouldn't even be able to attempt this move to begin with).

players.py

• the human player seems okay. There are a few minor points, but this post already is in TL;DR territory.


• I still can't find the code that belongs to get_enemy, I feel like I'm blind :D


• Your computer seems to modify the board during planning and you are passing in the actual board


• the minimax implementation is actually not thread safe and there is a lot that can be optimized here, but since TicTacToe is only a very small game it doesn't matter.

1. 
   

   Custom exceptions don't need an explicit __init__(). A simple

   
   
   

   class MyCustomError(ValueError):
    pass
   

   
   
   

   will do. Python will provide an implicit constructor to call the baseclass.

   
   



2. Type annotations can refer to user-defined types. (object, object) is pretty much useless, since all other types derive from it.




3. Use string formatting instead of string concatenation. It is clearer to read and most likely faster.




4. By directly casting input() to int, without checking if the response is numerical, if a user (acidentally) enters a non-numerical string, the program breaks and prints a pretty unhelpful traceback message. You could try catching a ValueError in a try: / except: block. If you find yourself repeating the same pattern many times, maybe write a get_integer() function, which repeatedly asks for input until a numerical response is given.




5. In a function, the else keyword can be left out if the if:-clause returns (or exits the program, for that matter). While it may seem insignificant, this can improve readability (less indentation).




6. Don't use wildcard imports. They clutter the global namespace and can easily cause name collisions. It may work for personal projects like these, but once you start doing the same with external libraries and frameworks, all hell breaks loose. Apart from avoiding name conflicts, by explicitly listing every item you want to import, other developers can see at a glance where an object comes from.




7. I would move the print() call from TicTacToe.__init__() to TicTacToe.run(). In my opinion, constructors should not be concerned with starting the game (and doing IO).