Design of a Tic Tac Toe program in Python

Question

Here is a program I wrote to reproduce the known win proportions for random Tic Tac Toe games (X always moving first). It takes about a minute to play the one million random games that will be played if you run main.py after putting all the below .py files in the same directory. (The program does reproduce the correct results.)

I'm studying design principles for keeping large code bodies structured for easier maintenance.

I'm trying to:

• focus on classes where everything is at the same level of abstraction
• make sure that everything in a class very clearly belongs in that class
• make it so that the reader can understand the current method he/she is reading without knowing what is happening anywhere else in the program

I'd appreciate some feedback on how I can improve on the above issues. The code should be a fast read. But to make it even faster, may I explain what I was thinking about each class? (main.py has no class of its own... it just gets the program going).

• For the GamesEngine class I was thinking that no force would cause games to play, moves to be made, or reports to be run, so the engine is the needed force.
• For the GameState class I tried to put only the minimum information needed to hold the current board and the relevant information the comes with the current board.
• For the GameRules class I tried to only put the rules that are needed to make conclusions about the current board (i.e. is there a win or a tie on the board).
• Finally, the ReportOnGame class prints the final board for a single game along with who won, while the ReportOnManyGames class finds the proportions of X wins, O wins, and ties over many games.

Any thoughts on design would be much appreciated. (And if you see any improvements that can be made that aren't design related I'd love to hear about them.)

main.py

from games_engine import GamesEngine
from report_on_game import ReportOnGame
from report_on_many_games import ReportOnManyGames

games_engine = GamesEngine()
report_on_game = ReportOnGame()
report_on_many_games = ReportOnManyGames()

# Pass True to set_end_of_game_reporting to report on each game
report_on_game.set_end_of_game_reporting(False)

# parameter passed to play_games determines the number of games that will be played
games_engine.play_game(1000000)

# shows percentage of wins from X and O as well as percentage of ties
report_on_many_games.report_outcome_statistics()

games_engine.py

import random
from game_state import GameState
from game_rules import GameRules
from report_on_game import ReportOnGame
from report_on_many_games import ReportOnManyGames

class GamesEngine:

    def __init__(self):
        self.game_state = GameState()
        self.game_rules = GameRules()
        self.report_on_game = None
        self.report_on_many_games = ReportOnManyGames()

    # responsible for playing the number of games requested from main.py
    def play_game(self, number_of_games):
        while number_of_games > 0:
            self.report_on_game = ReportOnGame()
            self.game_state = GameState()
            game_in_progress = True
            while game_in_progress:
                game_in_progress = self.make_move()
            number_of_games -= 1

    def make_move(self):
        # randomly select an available square from the available_squares set, and store as a list of length 1
        position_as_list_of_list = random.sample(self.game_state.available_squares, 1)
        row_index = position_as_list_of_list[0][0]
        column_index = position_as_list_of_list[0][1]
        # remove the available square we will shortly use from the available_squares list
        self.game_state.available_squares.remove([row_index, column_index])
        # make move
        self.game_state.board[row_index][column_index] = self.game_state.next_move
        # call game_over to see if the game is over
        if self.game_rules.game_over(self.game_state, row_index, column_index):
            self.between_game_reporting(self.game_rules.winning_letter)
            return False
        # update who moves next
        temp = self.game_state.next_move
        self.game_state.next_move = self.game_state.previous_move
        self.game_state.previous_move = temp
        return True

    def between_game_reporting(self, win_result='Tie'):

        if self.report_on_game.end_of_game_report:
            self.report_on_game.end_of_game_reporter(self.game_state, win_result)

        self.report_on_many_games.track_game_outcomes(win_result)

game_state.py

class GameState:

    def __init__(self):
        self.board = None
        self.available_squares = None
        self.initialize_board_and_available_squares()
        self.next_move = 'X'
        self.previous_move = 'O'

    def initialize_board_and_available_squares(self):
        self.board = [[' ' for i in range(3)] for j in range(3)]
        self.available_squares = [[i, j] for i in range(3) for j in range(3)]

    # Printing an instance of the class will display a standard
    # tic tac toe game image.
    def __str__(self):
        return "\n" + self.board[0][0] + "|" + self.board[0][1] + "|" + self.board[0][2] + "\n"+ self.board[1][0] + "|" + self.board[1][1] + "|" + self.board[1][2] + "\n" + self.board[2][0] + "|" + self.board[2][1] + "|" + self.board[2][2]

game_rules.py

class GameRules:

    def __init__(self):
        self.letter_dict = {'X': -1, 'O': 1, ' ': 0}
        self.winning_letter = None

    def game_over(self, game_state, row_index, column_index):

        # check row containing most recent move for win
        total = 0
        for column in range(3):
            total = total + int(self.letter_dict[game_state.board[row_index][column]])
            if abs(total) == 3:
                self.winning_letter = game_state.board[row_index][column_index]
                return True

        # check column containing most recent move for win
        total = 0
        for row in range(3):
            total = total + int(self.letter_dict[game_state.board[row][column_index]])
            if abs(total) == 3:
                self.winning_letter = game_state.board[row_index][column_index]
                return True

        # check for win on main-diagonal if it contains most recent move
        if row_index == column_index:
            total = 0
            for diagonal_indexing in range(3):
                total = total + int(self.letter_dict[game_state.board[diagonal_indexing][diagonal_indexing]])
                if abs(total) == 3:
                    self.winning_letter = game_state.board[row_index][column_index]
                    return True

        # check for win on off-diagonal if it contains most recent move
        if row_index + column_index == 2:
            total = 0
            for off_diagonal_indexing in range(3):
                total = total + int(self.letter_dict[game_state.board[off_diagonal_indexing][2 - off_diagonal_indexing]])
                if abs(total) == 3:
                    self.winning_letter = game_state.board[row_index][column_index]
                    return True

        if len(game_state.available_squares) == 0:
            self.winning_letter = 'Tie'
            return True

        return False

report_on_game.py

class ReportOnGame:

    end_of_game_report = False

    @classmethod
    def set_end_of_game_reporting(cls, boolean_set):
        cls.end_of_game_report = boolean_set

    # call turn_on_end_of_game_reporting from main.py to run this report method
    @staticmethod
    def end_of_game_reporter(board, result='Its a tie'):
        print(board)
        if result == 'X':
            print(result + ' won\n')
        elif result == 'O':
            print(result + ' won\n')
        else:
            print(result + '\n')

report_on_many_games.py

class ReportOnManyGames:

    number_of_x_wins = 0
    number_of_o_wins = 0
    number_of_ties = 0

    @classmethod
    def track_game_outcomes(cls, win_result):
        if win_result == 'X':
            cls.number_of_x_wins = cls.number_of_x_wins + 1
        if win_result == 'O':
            cls.number_of_o_wins = cls.number_of_o_wins + 1
        if win_result == 'Tie':
            cls.number_of_ties = cls.number_of_ties + 1

    @classmethod
    def report_outcome_statistics(cls):
        total_games = cls.number_of_x_wins + cls.number_of_o_wins + cls.number_of_ties
        print('Proportion of X wins: ' + str(cls.number_of_x_wins/total_games))
        print('Proportion of O wins: ' + str(cls.number_of_o_wins / total_games))
        print('Proportion of ties: ' + str(cls.number_of_ties / total_games))

Answer 1

While your goal may be to "make it so the reader can understand the current method they are reading", I actually found it very confusing to read the code.

Consider:

from games_engine import GamesEngine
from report_on_game import ReportOnGame
from report_on_many_games import ReportOnManyGames

Ok, so games_engine, report_on_game, and report_on_many_games all modules, but only one class from each module is imported to the top-level namespace; the modules themselves are not added to the top-level namespace. Later:

report_on_game.set_end_of_game_reporting(False)
games_engine.play_game(1000000)
report_on_many_games.report_outcome_statistics()

Wait. None of those modules were imported to the top-level namespace. What is going on??? Confusion!!! This continues with self.report_on_game and self.game_state and self.game_rules and so on. Is report_on_game a module name, a global variable name, or a class member??? The answer is all three! This same name different semantics does not help readability.

One class per file is not the Pythonic way of doing things, and as you can see, it is causing naming confusion. I'd recommend one file ... maybe tic_tac_toe.py with all 5 of your classes in that one file, and perhaps even the main function

---

GameEngine.play_game doesn't play a game. It plays a bunch of games. Separate this into two functions: one to play a game, and one which repeatedly calls the former to play multiple games.

def play_game(self):

    self.game_state = GameState()
    game_in_progress = True

    while game_in_progress:
        game_in_progress = self.make_move()

    # Maybe add end-of-game reporting here.


def play_games(self, number_of_games):

    for _ in range(number_of_games):
        self.play_game()

    # Maybe add end-of-all-games reporting here

---

Why does self.make_move() return game_in_progress? Perhaps GameState should contain the in_progress member. After all, whether the game is over (and even who won!), is part of the game state!

def play_game(self):
    self.game_state = GameState()

    while self.game_state.in_progress:
        self.make_move()

    # Maybe add end-of-game reporting here.

---

make_move() should really be called make_random_move() to be clear that it is just making random moves.

---

GameRules.game_over() is both returning a value directly, and returning a value through an evil, secret, underhanded modifying self.winning_letter state variable.

It could simply return one of 4 values: "X", "O", "Tie" or "". An empty string, which is a False-ish value would indicate the game is not yet over, while the other three True-ish values would simultaneously indicate the game is over as well as who won.

---

You are confusing classes for method containers, and creating classes with no data.

Consider ReportOnGame. It has no per-instance data, just two static methods and a class variable. There is never a reason to create a ReportOnGame() instance because it doesn't contain any data, and yet you create 1 million instances of it.

Yes, you do access self.report_on_game.end_of_game_report and call self.report_on_game.end_of_game_reporter(...), which appears to be using an instance of the ReportOnGame() object, but since that instance has no instance data, you could just as easily wrote:

if ReportOnGame.end_of_game_report:
    ReportOnGame.end_of_game_reporter(self.game_state, win_result)

and never created the objects.

But it doesn't end there. The .end_of_game_report is a global variable contained in different class in a module from GameEngine. Why does that foreign class & module have a flag that controls what GameEngine does?

It would be (slightly) better for ReportOnGame to internally check the flag when reporting:

class ReportOnGame:
    end_of_game_report = False
    #...
    @staticmethod
    def end_of_game_reporter(board, result='Its a tie'):
        if ReportOnGame.end_of_game_report:
             print(board)
             # ... etc ...

And then you could unconditionally call ReportOnGame.end_of_game_reporter(self.game_state, win_result) in GameEngine, without needing to reach into another class in another module to access the global variable.

But ReportOnGame.end_of_game_reporter(...) is still just a static function; it does not warrant a class.

---

How would I re-write this?

As I mentioned above, it would be inside a single tic_tac_toe.py module.

End of game reporting would simply be a function:

def end_of_game_report(board, result):
    print(board)
    if result != 'Tie':
        print(result + ' won.')
    else:
        print("Tie game.")

GameEngine could have a reporters member, containing a list of methods to call to report the game result:

class GameEngine:
    def __init__(self, *reporters):
        self._reporters = reporters

engine = GameEngine(end_of_game_report)
engine.play(5)

After each game is over, the engine would need to call all of the reporters, passing in the board state and result.

for reporter in self._reporters:
    reporter(self._board, win_result)

And if you don't want a report after each game, you simply don't need to pass end_of_game_report in the list of reporters. If it is not in the list, it doesn't get called. No boolean flags required.

As for ReportOnManyGames, the 3 static variables can be replaced with a counter dictionary. An object of that class can be made callable, so it can be used as a reporter, above.

class ReportOnManyGames:

    def __init__(self):
        self._counts = { 'X':0, 'O':0, 'Tie':0 }

    def __call__(self, board, result):
        self._counts[result] += 1

    def report_outcome_statistics(self):
        total_games = sum( self._counts.values() )

        print('Proportion of X wins: {:5.2f}%'.format(self._counts['X'] / total_games * 100))
        # ... etc ...

Now you can create a callable object which keeps track of the total wins and ties.

summarizer = ReportOnManyGames()

engine = GameEngine(summarizer)    # omit end_of_game_report, since we have a million games
engine.play(1_000_000)

summarizer.report_outcome_statistics()