Python TicTacToe

Question

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)

Answer 1

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.

Answer 2

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).