Design Tic Tac Toe Interview Problem [Python 3]

Question

Practicing objected oriented design. This time, it's designing the TicTacToe game. One difference between the real TicTacToe game is that I've made my TicTacToe game to have variable sized board.

Please review in terms of OOD and I've also left some comments in my code where I want feedback in particular. Thank you very much in advance.

Misc

from enum import Enum
from typing import Union, Optional, Callable
class InvalidInput(Exception):
 pass
class SpotTaken(Exception):
 def __init__(self, pos_x: int, pos_y: int) -> None:
 super().__init__(f'({pos_x}, {pos_y}) is already taken!')
class GameNotRunning(Exception):
 def __init__(self) -> None:
 super().__init__('Game is currently not running.')
class InvalidBoardSize(Exception):
 def __iter__(self) -> None:
 super().__init__('Board size must be a positive integer.')

TicTacToe

class TicTacToe:
 DEFAULT_BOARD_SIZE = 3
 # Should this be inside the TicTacToe class or outside?
 class Turn(str, Enum):
 X = 'X'
 O = 'O'
 def __str__(self) -> str:
 return self.value
 # Should this be inside the TicTacToe class or outside?
 class State(Enum):
 RUNNING = 0
 TURN_WON = 1
 TIE = 2
 def __init__(self, board_size: Optional[int] = None) -> None:
 if board_size is not None and board_size <= 0:
 raise InvalidBoardSize()
 self.board_size = board_size or self.DEFAULT_BOARD_SIZE
 self.board: list[list[Union[None, str]]] = [[None] * self.board_size for _ in range(self.board_size)]
 self.turn = self.Turn.X
 self.state = self.State.RUNNING
 self.spots_left = self.board_size * self.board_size
 def do_turn(self, pos_x: int, pos_y: int) -> None:
 if self.state != self.State.RUNNING:
 raise GameNotRunning()
 if pos_x < 0 or pos_x >= self.board_size or pos_y < 0 or pos_y >= self.board_size:
 raise InvalidInput('pos_x or pos_y is out of bounds!')
 if self.board[pos_x][pos_y]:
 raise SpotTaken(pos_x=pos_x, pos_y=pos_y)
 self.board[pos_x][pos_y] = self.turn
 self.spots_left -= 1
 self._check_state()
 if self.state == self.State.RUNNING:
 self._change_turn()
 # Optimally, I don't have to check all 8 possible ways.
 # Technically, I only need to check the vertical of the column pos_y, the horizontal of the row pos_x,
 # and the diagonals if the (pos_x, pos_y) overlaps with any of the points on the diagonal path.
 # However, just to have a simpler code, just do the check on all 8 ways always.
 # Since, these are such simple operations, it should not affect the performance really, anyway.
 def _check_state(self) -> None:
 # vertical check
 for row in range(self.board_size):
 winning_line_found = True
 for col in range(self.board_size):
 if self.board[row][col] != self.turn:
 winning_line_found = False
 break
 if winning_line_found:
 self.state = self.State.TURN_WON
 return
 # horizontal check
 for col in range(self.board_size):
 winning_line_found = True
 for row in range(self.board_size):
 if self.board[row][col] != self.turn:
 winning_line_found = False
 break
 if winning_line_found:
 self.state = self.State.TURN_WON
 return
 # top left to bottom right diagonal check
 winning_line_found = True
 for i in range(self.board_size):
 if self.board[i][i] != self.turn:
 winning_line_found = False
 if winning_line_found:
 self.state = self.State.TURN_WON
 return
 # top right to bottom left diagonal check
 winning_line_found = True
 for i in range(self.board_size):
 if self.board[i][self.board_size - i - 1] != self.turn:
 winning_line_found = False
 if winning_line_found:
 self.state = self.State.TURN_WON
 return
 if self.spots_left == 0:
 self.state = self.State.TIE
 def _change_turn(self) -> None:
 self.turn = self.Turn.X if self.turn == self.Turn.O else self.Turn.O
 def _get_status_msg(self) -> str:
 if self.state == self.State.RUNNING:
 msg = f'{self.turn}\'s turn.'
 elif self.state == self.State.TURN_WON:
 msg = f'{self.turn} WON!'
 else:
 msg = 'It\'s a TIE'
 return msg
 def _get_board_display(self) -> str:
 return '\n'.join(str([str(val) if val else ' ' for val in row]) for row in self.board)
 def __str__(self) -> str:
 return (f'{self._get_board_display()}\n'
 f'{self._get_status_msg()}')

TicTacToe CLI Manager

class TicTacToeCLIManager:
 class State(Enum):
 DETERMINE_BOARD_SIZE = 0
 RUN_GAME = 1
 ASK_RESTART = 2
 QUIT = 3
 def __init__(self) -> None:
 self.tic_tac_toe: Union[TicTacToe, None] = None
 self.state = self.State.DETERMINE_BOARD_SIZE
 def run(self) -> None:
 while self.state != self.State.QUIT:
 self.STATE_TO_HANDLER[self.state](self)
 def _handle_determine_board_size_state(self) -> None:
 print('Determine the size of the board')
 board_size = None
 while board_size is None:
 user_input = input()
 try:
 board_size = int(user_input)
 except ValueError:
 print('The size of the board must be an integer')
 try:
 self.tic_tac_toe = TicTacToe(board_size=board_size)
 except InvalidBoardSize:
 print('The size of the board has to be a positive integer')
 return
 print(self.tic_tac_toe)
 self.state = self.State.RUN_GAME
 def _handle_run_game_state(self) -> None:
 user_input = input()
 user_input_split = user_input.split(' ')
 try:
 pos_x, pos_y = int(user_input_split[0]), int(user_input_split[1])
 except (ValueError, IndexError):
 print('Inputs must have at least two integers separated by a space')
 print(self.tic_tac_toe)
 return
 try:
 self.tic_tac_toe.do_turn(pos_x=pos_x, pos_y=pos_y)
 except (InvalidInput, SpotTaken):
 print('Pick the position correctly, please')
 print(self.tic_tac_toe)
 if self.tic_tac_toe.state != self.tic_tac_toe.State.RUNNING:
 self.state = self.State.ASK_RESTART
 def _handle_ask_restart_state(self) -> None:
 print('Do you want to replay? If so, type y')
 user_input = input()
 if user_input == 'y':
 self.state = self.State.DETERMINE_BOARD_SIZE
 else:
 self.state = self.State.QUIT
 # I want to put this constant STATE_TO_HANDLER at the top of the class. I don't like constants being at the bottom.
 # However, if I put this above these methods, it cannot find the references to the methods.
 # What's the best practice regarding this?
 #
 # I wanted to put typing on the Callable's parameter and return types. Eg. Callable[[TicTacToeCLIManager], None]
 # However, the parameter type would be TicTacToeCLIManager and it cannot find the reference here
 # since the TicTacToeCLIManager definition is not finished.
 # What's the best practice regarding this, too?
 STATE_TO_HANDLER: dict[State, Callable] = {
 State.DETERMINE_BOARD_SIZE: _handle_determine_board_size_state,
 State.RUN_GAME: _handle_run_game_state,
 State.ASK_RESTART: _handle_ask_restart_state,
 }

runner

def test():
 TicTacToeCLIManager().run()

Answer 1

This is an incomplete review up to the function TicTacToe.turn

---

Re: if pos_x < 0 or pos_x >= self.board_size or pos_y < 0 or pos_y >= self.board_size:

An alternative to if x < min_x or x > max_x is if not min_x <= x <= max_x. I think the latter is more readable, but that's up to opinion. In this case, if you wanted to make the change, you'd change if pos_x < 0 or pos_x >= self.board_size to if not 0 <= pos_x < self.board_size and likewise for pos_y.

---

Suggest you rename self._check_state() to self._update_state() or something that indicates that the function might modify state.

---

# Optimally, I don't have to check all 8 possible ways.

This comment is only valid for a board size of 3. In general, an N x N board would have 2N + 2 possible "ways", so it might be beneficial to implement the optimization you mentioned. I.e. this optimization:

# Technically, I only need to check the vertical of the column pos_y, the horizontal of the row pos_x,
 # and the diagonals if the (pos_x, pos_y) overlaps with any of the points on the diagonal path.

---

Horizontal check

 # vertical check
 for row in range(self.board_size):
 winning_line_found = True
 for col in range(self.board_size):
 if self.board[row][col] != self.turn:
 winning_line_found = False
 break

If you consider rows to be horizontal and columns to be vertical, then this is actually a # horizontal check

Second, the code can be simplified to

horiz_winning_line_found = any(row == [self.turn] * self.board_size for row in self.board)

or, better yet, if y_pos is passed into self._check_state as an argument, you can just do

horiz_winning_line_found = self.board[y_pos] == [self.turn] * self.board_size

If you decide to go with this, it would make sense to make [self.Turn.X] * self.board_size and [self.Turn.O] * self.board_size into class variables of the TicTocToe class.

---

This not the most OOP thing to do, but if you consider a different representation of X and O on the board where X is marked by -1 and O is marked by +1, then the check state code could be simplified further. For example,

horiz_winning_line_found = self.board[y_pos] == [self.turn] * self.board_size

would become

horiz_winning_line_found = abs(sum(self.board[y_pos])) == self.board_size

also self._change_turn would simplify to

def _change_turn(self) -> None:
 self.turn *= -1

---

Vertical check

If you pass x_pos into self._check_state, then you can simplify the vertical winning line check to just

vert_winning_line_found = all(self.board[r][x_pos] == self.turn for r in range(self.board_size))

or

vert_winning_line_found = ''.join(self.board[r][x_pos] for r in range(self.board_size) == [self.turn] * self.board_size

---

You are mixing terms like (rows, columns) with (x_pos, y_pos). I would suggest you rename (x_pos, y_pos) to (c, r), (col, row), (colNum, rowNum) or something to that effect so that everything is in terms of rows and columns.

---

If you do end up passing x_pos and y_pos to self._check_state, here's a quick way to check whether x_pos, y_pos lie on the diagonal

# NW to SE diagonal
check_main_diag = x_pos == y_pos

# SW to NE diagonal
check_minor_diag = x_pos == self.board_size - 1 - y_pos

then you can just do

main_diag_winning_line = check_main_diag * all(self.board[i][i] == self.turn for i in range(self.board_size))

and something similar for minor_diag_winning_line

• whiteSkar: Awesome ways to check the result! Thank you very much.