Tic-tac-toe in python with OOP

Question

I'm a beginner to intermediate in programming, and created a Tic-tac-toe game in python. I plan to add an AI player and a GUI using pygame.

Any improvements I could make to the code, especially with the logic, or just formatting in general?

my_board = [
 ["1", "2", "X"],
 ["4", "5", "6"],
 ["7", "8", "9"]
]
class Tictactoe():
 def __init__(self, board):
 self.board = board
 
 def print_board(self):
 print("|-------------|")
 print("| Tic Tac Toe |")
 print("|-------------|")
 print("| |")
 print("| " + self.board[0][0] + " " + self.board[0][1] + " " + self.board[0][2] + " |")
 print("| " + self.board[1][0] + " " + self.board[1][1] + " " + self.board[1][2] + " |")
 print("| " + self.board[2][0] + " " + self.board[2][1] + " " + self.board[2][2] + " |")
 print("| |")
 print("|-------------|")
 print()
 def available_moves(self):
 moves = []
 for row in self.board:
 for col in row:
 if col != "X" and col != "O":
 moves.append(int(col))
 return moves
 def select_space(self, move, turn):
 row = (move-1)//3
 col = (move-1)%3
 self.board[row][col] = "{}".format(turn)
 def has_won(self, player):
 for row in self.board:
 if row.count(player) == 3:
 return True
 for i in range(3):
 if self.board[0][i] == player and self.board[1][i] == player and self.board[2][i] == player:
 return True
 if self.board[0][0] == player and self.board[1][1] == player and self.board[2][2] == player:
 return True
 if self.board[0][2] == player and self.board[1][1] == player and board[2][0] == player:
 return True
 return False
 
 def game_over(self):
 self.available_moves()
 if self.has_won("X") or self.has_won("O") or len(self.available_moves()) ==0:
 return True
 else:
 return False
 def get_player_move(self, turn):
 move = int(input("Player {player} make your turn: ".format(player=turn)))
 self.available_moves()
 while move not in range (1,10) or move not in self.available_moves():
 move = int(input("Invalid move, try again: "))
 return move
 
 
def game(board):
 tictactoe = Tictactoe(my_board)
 while not tictactoe.game_over():
 tictactoe.print_board()
 x_move = tictactoe.get_player_move("X")
 tictactoe.select_space(x_move,"X")
 tictactoe.print_board()
 if tictactoe.game_over():
 break
 o_move = tictactoe.get_player_move("O")
 tictactoe.select_space(o_move,"O")
 if tictactoe.has_won("X"):
 print("X wins")
 elif tictactoe.has_won("O"):
 print("O wins")
 else:
 print("tie")
game(my_board)

Answer 1

---

Indents

PEP 8: Use 4 spaces per indentation level.

---

Main game loop

The way your game loop is set up now is a bit unelegant. You already implemented all the methods to be able to handle both players, there's only one piece missing:

from itertools import cycle
players = cycle(['X', 'O'])

cycle will provide a generator that infinitely repeats the iterable you pass it. This makes the loop quite a bit cleaner:

while not tictactoe.game_over():
 tictactoe.print_board()
 player = next(players)
 move = tictactoe.get_player_move(player)
 tictactoe.select_space(move, player)

I'd also suggest renaming select_space to something like do_move. Makes it a bit clearer what's going on.

---

Redundant calls to Tictactoe.available_moves()

In game_over and get_player_move you call self.available_moves() once without using the return value in any way. You can simply delete those lines.

---

game_over

The pattern

if self.has_won("X") or self.has_won("O") or len(self.available_moves()) ==0:
 return True
else:
 return False

can be simplified to

return self.has_won("X") or self.has_won("O") or len(self.available_moves()) == 0

What you were doing is basically manually returning the resulting bool value of a condition. We can instead simply return the condition directly.

---

print_board

Python lets you easily avoid an unelegant pattern

print("| " + self.board[0][0] + " " + self.board[0][1] + " " + self.board[0][2] + " |")

by using the * operator for unpacking an iterable

print("| ", *self.board[0], " |")

Since we need to do this for all rows in self.board we should let Python handle it for us as well:

for row in self.board:
 print("| ", *row, " |")

---

has_won

For a cleaner implementation of this method I would suggest implementing helper functions that provide lists of the rows, columns and diagonals of the board. They might also be useful for future feature additions.

def rows(self):
 return self.board[:] # creates a mutable copy of the rows
def columns(self):
 return [[row[index] for row in self.board] for index in range(3)]
def diagonals(self):
 first = [self.board[index][index] for index in range(3)]
 second = [self.board[index][len(self.board[index]) - index - 1] for index in range(3)]
 return [first, second]

These methods are also ready to be used for boards of different sizes. Simply replace the 3s by a variable.

has_won is now a lot simpler and more readable:

def has_won(self, player):
 for line in self.rows() + self.columns() + self.diagonals():
 symbols = set(line)
 if player in symbols and len(symbols) == 1:
 return True
 return False

We can take this a step further (as pointed out in the comments) and change has_won to get_winner and return the winning symbol. This will also affect some other parts of your code:

class Tictactoe:

def get_winner(self):
 for line in self.rows() + self.columns() + self.diagonals():
 symbols = set(line)
 if len(symbols) == 1:
 return symbols.pop()
 return None
def game_over(self):
 return self.get_winner() is not None or len(self.available_moves()) == 0

Printing our winner in game:

def game(...):
 
 # main gameplay here
 
 if (winner := self.get_winner()) is not None:
 print(f"{winner} wins")
 else:
 print("tie")

• FMc: Plenty of good advice here. In addition, the OP could consider another simplification of has_won(): unless I've overlooked something, it doesn't need the player argument: if all symbols are the same (and not empty), someone has one; just return who it is.
• riskypenguin: Very good suggestion, I added it to my answer!

Answer 2

one performance improvement suggestion

available_moves

Instead of iterating through all the moves create a set with all the moves in the beginning and as each player makes a move remove the corresponding move from the set.

In your code both iterating through all moves to find the valid moves(available_moves function) and then the checking if the move given is valid (move not in self.available_moves()) both of this are o(n) operations. But using set I believe both these operation could average to o(1).

Might not be a huge difference in performance considering tic tac toe only has 9 moves. But might be a good way to tackle the problems in other similar situations.