Tic Tac Toe using the __init__ constructor

Question

I recently learned about classes in python. I only have a brief understanding of them, but I think it's good enough for me to write a tic tac toe program (been working on this for the last week or so). My title may be a little misleading (again, because I only have a brief understanding...) but I think you'll get the point when you see my code. I don't have any concerns with my code, but if you have any suggestions/criticism please don't hesitate to let me know.

my code:

from tkinter import *
import numpy as np

size_of_board = 600
size_of_symbol = (size_of_board / 3 - size_of_board / 8) / 2
thickness_of_symbol = 50
colour_of_X = '#FFB6C1'
colour_of_O = '#2ADCCB'
score_board_colour = '#8A2BE2'


class Tic_Tac_Toe():

    # ------------------------------------------------------------------
    # initialize functions:
    # ------------------------------------------------------------------

    def __init__(self):
        self.window = Tk()
        self.window.title('Tic-Tac-Toe')
        self.canvas = Canvas(self.window, width = size_of_board, height = size_of_board)
        self.canvas.pack()
        # input from user in the form of clicks
        self.window.bind('<Button-1>', self.click)

        self.initialize_board()
        self.player_X_turns = True
        self.board_status = np.zeros(shape = (3, 3))

        self.player_X_starts = True
        self.reset_board = False
        self.gameover = False
        self.tie = False
        self.X_wins = False
        self.O_wins = False

        self.X_score = 0
        self.O_score = 0
        self.tie_score = 0

    def mainloop(self):
        self.window.mainloop()

    def initialize_board(self):
        for i in range(2):
            self.canvas.create_line((i + 1) * size_of_board / 3, 0, (i + 1) * size_of_board / 3, size_of_board)

        for i in range(2):
            self.canvas.create_line(0, (i + 1) * size_of_board / 3, size_of_board, (i + 1) * size_of_board / 3)

    def play_again(self):
        self.initialize_board()
        self.player_X_starts = not self.player_X_starts
        self.player_X_turns = self.player_X_starts
        self.board_status = np.zeros(shape = (3, 3))

    # ------------------------------------------------------------------
    # the modules required to draw required game based object on canvas
    # ------------------------------------------------------------------

    def draw_O(self, logical_position):
        logical_position = np.array(logical_position)
        # logical_position = grid value on the board
        # grid_position = actual pixel values of the center of the grid
        grid_position = self.convert_logical_to_grid_position(logical_position)
        self.canvas.create_oval(grid_position[0] - size_of_symbol, grid_position[1] - size_of_symbol,
                                grid_position[0] + size_of_symbol, grid_position[1] + size_of_symbol,
                                width = thickness_of_symbol, outline = colour_of_O)

    def draw_X(self, logical_position):
        grid_position = self.convert_logical_to_grid_position(logical_position)
        self.canvas.create_line(grid_position[0] - size_of_symbol, grid_position[1] - size_of_symbol,
                                grid_position[0] + size_of_symbol, grid_position[1] + size_of_symbol,
                                width = thickness_of_symbol, fill = colour_of_X)
        self.canvas.create_line(grid_position[0] - size_of_symbol, grid_position[1] + size_of_symbol,
                                grid_position[0] + size_of_symbol, grid_position[1] - size_of_symbol,
                                width = thickness_of_symbol, fill = colour_of_X)

    def display_gameover(self):

        if self.X_wins:
            self.X_score += 1
            text = 'Player 1 (X) has won'
            colour = colour_of_X
        elif self.O_wins:
            self.O_score += 1
            text = 'Player 2 (O) has won'
            colour = colour_of_O
        else:
            self.tie_score += 1
            text = 'Draw!!'
            colour = 'coral'

        self.canvas.delete('all')
        self.canvas.create_text(size_of_board / 2, size_of_board / 3, font = 'cmr 40 bold', fill = colour, text = text)

        score_text = 'Scores \n'
        self.canvas.create_text(size_of_board / 2, 5 * size_of_board / 8, font = 'cmr 30 bold', fill =
        score_board_colour, text = score_text)

        score_text = 'Player 1 (X) : ' + str(self.X_score) + '\n'
        score_text += 'Player 2 (O) : ' + str(self.O_score) + '\n'
        score_text += 'Tie               : ' + str(self.tie_score)
        self.canvas.create_text(size_of_board / 2, 3 * size_of_board / 4, font = 'cmr 20 bold', fill =
        score_board_colour, text = score_text)
        self.reset_board = True

        score_text = 'Click to play again \n'
        self.canvas.create_text(size_of_board / 2, 15 * size_of_board / 16, font = 'cmr 10 bold', fill = 'orange',
        text = score_text)

    # ------------------------------------------------------------------
    # the modules required to carry out game logic
    # ------------------------------------------------------------------

    def convert_logical_to_grid_position(self, logical_position):
        logical_position = np.array(logical_position, dtype = int)
        return (size_of_board / 3) * logical_position + size_of_board / 6

    def convert_grid_to_logical_position(self, grid_position):
        grid_position = np.array(grid_position)
        return np.array(grid_position // (size_of_board / 3), dtype = int)

    def is_grid_occupied(self, logical_position):
        if self.board_status[logical_position[0]][logical_position[1]] == 0:
            return False
        else:
            return True

    def is_winner(self, player):

        player = -1 if player == 'X' else 1

        # three in a row
        for i in range(3):
            if self.board_status[i][0] == self.board_status[i][1] == self.board_status[i][2] == player:
                return True
            if self.board_status[0][i] == self.board_status[1][i] == self.board_status[2][i] == player:
                return True

        # diagonals
        if self.board_status[0][0] == self.board_status[1][1] == self.board_status[2][2] == player:
            return True

        if self.board_status[0][2] == self.board_status[1][1] == self.board_status[2][0] == player:
            return True

        return False

    def is_tie(self):

        r, c = np.where(self.board_status == 0)
        tie = False
        if len(r) == 0:
            tie = True

        return tie

    def is_gameover(self):
        # either someone is declared the winner or the entire grid is occupied
        self.X_wins = self.is_winner('X')
        if not self.X_wins:
            self.O_wins = self.is_winner('O')

        if not self.O_wins:
            self.tie = self.is_tie()

        gameover = self.X_wins or self.O_wins or self.tie

        if self.X_wins:
            print('X wins')
        if self.O_wins:
            print('O wins')
        if self.tie:
            print('It\'s a tie')

        return gameover


    def click(self, event):
        grid_position = [event.x, event.y]
        logical_position = self.convert_grid_to_logical_position(grid_position)

        if not self.reset_board:
            if self.player_X_turns:
                if not self.is_grid_occupied(logical_position):
                    self.draw_X(logical_position)
                    self.board_status[logical_position[0]][logical_position[1]] = -1
                    self.player_X_turns = not self.player_X_turns
            else:
                if not self.is_grid_occupied(logical_position):
                    self.draw_O(logical_position)
                    self.board_status[logical_position[0]][logical_position[1]] = 1
                    self.player_X_turns = not self.player_X_turns

            # check if the game has concluded
            if self.is_gameover():
                self.display_gameover()
                # print('Done')
        else:  # Play Again
            self.canvas.delete('all')
            self.play_again()
            self.reset_board = False

play_game = Tic_Tac_Toe()
play_game.mainloop()

Answer 1

The main problem I see is how closely tied to the UI the game logic is. You have one class that's responsible for both handling the UI (initialize_board, display_gameover, draw_X) and handing game logic (is_winner, is_tie, is_grid_occupied). This means that in order to test the logic of the game, you need to construct a Tic_Tac_Toe, which will launch the UI.

I'd break the game logic off into its own class that wraps the numpy array, and exposes game-related methods (place_x, place_o, is_winner). That way you can manipulate the game object easily using automated testing, completely separately from UI. You would then give the main "application class" an instance of the game for it to manipulate and query as needed.

---

For the same, but lesser reasons, you may also want to directly wrap the numpy array in a Board class to clean up reads/writes to the board in your game class. Adding helper methods that allow for setting using tuples could clean up lines like this:

self.board[logical_position[0]][logical_position[1]] = 1

A set_at method that accepts a tuple/sequence could replace that:

self.board.set_at(logical_position, 1)

Which reads much nicer. A similar get_at method could be created for lookups that use sequences as well.

---

is_grid_occupied is more verbose than it needs to be. Using a conditional statement to dispatch to True/False is redundant since the condition already evaluates to the same values. You can just have:

def is_grid_occupied(self, logical_position):
    return self.board[logical_position[0]][logical_position[1]] == 0

Or, if you implement a get_at method:

def is_grid_occupied(self, logical_position):
    return self.board.get_at(logical_position) == 0

You could also negate the condition instead of comparing against 0, but I'd keep it like this for explicitness.

---

def is_winner(self, player):
    player = -1 if player == 'X' else 1

There's a couple things to note here:

• You're using -1 and 1 as placeholder values to indicate the players. These are examples of Magic Numbers, and should ideally be replaced by a more descriptive name. I'm not sure what the limitation on numpy arrays are, but an Enum of something like:

  from enum import Enum
  
  class Cell(Enum):
      X = -1
      O = 1
      EMPTY = 0
  

  would allow you to make more sensical code:

  self.board_status[logical_position[0]][logical_position[1]] = Cell.X
  . . .
  self.board_status[logical_position[0]][logical_position[1]] = Cell.O
  . . .
  player = Cell.X if player == 'X' else Cell.O
  

  And you could do away with that first line of is_winner altogether by just using the enum when calling it:

  self.X_wins = self.is_winner(Cell.X)
  

  If numpy arrays can't hold enums, try using an IntEnum instead of Enum, or just have global constants:

  PLAYER_X = -1
  PLAYER_O = 1
  EMPTY = 0
  

  And use those instead. You could also just switch to using normal Python lists. I don't think numpy is necessary here.

• I usually try to avoid needlessly reassigning variables, especially parameters. It's often handy to see what data was previously when print-debugging/sitting at a breakpoint in a debugger. I prefer to give it a name that describes the new state, or distinguishes it from the previous state. Since the string being passed in is a formatted string representation, I might rename it:

      def is_winner(self, formatted_player):
          player = Cell.X if formatted_player == 'X' else Cell.O
  

---

I realized after that it is actually possible to instantiate the class without actually launching the UI. I would still recommend splitting the separate functionality off though, for the sake of readability and maintenance. The less code you have to sift through when trying to pin down a bug or understand a section of code, the better.