Python text-based Tic Tac Toe

Question

I created a simple text-based Tic Tac Toe game in Python using OOP. Currently the computer moves are completely random - I plan to add some sort of algorithm later (no idea how to do that yet though)

Any suggestions about how to improve it are welcome.

from random import randint
from itertools import cycle
class Board:
 def __init__(self):
 self._board = [["-"]*3 for i in range(3)]
 def display(self):
 for row in self._board:
 for tile in row:
 if tile != "-":
 tile = tile.symbol
 print(tile, end=" ")
 print()
 def place_symbol(self, player, tile):
 """Try to place the player inside the tile
 The important thing here is that it returns None if it fails
 """
 row, colmn = tile
 if self._board[row][colmn] == "-":
 self._board[row][colmn] = player
 return True
 def check_win(self):
 """Checks all possible winning combinations,
 Returns True for a win and False otherwise.
 """
 # Store all checks here
 checks = set()
 # Add rows
 for row in self._board:
 checks.add(tuple(row))
 # Add columns
 colmns = zip(self._board[0], self._board[1], self._board[2])
 for colmn in colmns:
 checks.add(tuple(colmn))
 # Add diagonals
 diag1 = (self._board[0][0], self._board[1][1], self._board[2][2])
 diag2 = (self._board[0][2], self._board[1][1], self._board[2][0])
 checks.update((diag1, diag2))
 # Check every option for a win
 checks = {True if (len(set(lst)) == 1 and lst[0] != "-") else False for lst in checks}
 if True in checks:
 return True
 return False
 def is_full(self):
 if "-" not in (self._board[0]+self._board[1]+self._board[2]):
 return True
 return False
class Player:
 def __init__(self, is_human, symbol, name):
 self.is_human = is_human
 self.symbol = symbol
 self.name = name
 self.score = 0
def get_player_input(choices, text=''):
 while True:
 inpt = input(text)
 if inpt in choices:
 return inpt
 print(f"Enter one of the following: {', '.join(choices)}")
def main():
 print("Welcome to tic tac toe!")
 print("type the appropiate number to choose a game option:")
 print("1.player vs player\n2.player vs computer\n3.computer vs computer")
 choice = get_player_input(('1', '2', '3'),)
 if choice == '1':
 player1_name = input("Choose a Name for player 1: ")
 player2_name = input("Choose a Name for player 2: ")
 player1_is_human = True
 player2_is_human = True
 elif choice == '2':
 player1_name = input("Choose a name: ")
 player2_name = "Computer"
 player1_is_human = True
 player2_is_human = False
 elif choice == '3':
 player1_name = "Computer 1"
 player2_name = "Computer 2"
 player1_is_human = False
 player2_is_human = False
 player1 = Player(player1_is_human, "X", player1_name)
 player2 = Player(player2_is_human, "O", player2_name)
 players = [player1, player2]
 board = Board()
 # For player row and colmn input
 options = ('1', '2', '3')
 for player in cycle(players):
 board.display()
 print(f"It's {player.name}'s turn")
 # The actual turn of the player
 while True:
 if player.is_human:
 row = int(get_player_input(options, "Enter row number(1-3): ")) - 1
 colmn = int(get_player_input(options, "Enter column number(1-3): ")) - 1
 else:
 row, colmn = randint(0, 2), randint(0, 2)
 result = board.place_symbol(player, (row, colmn))
 if result is None:
 if player.is_human:
 print("Enter in a non-full tile")
 continue
 else:
 break
 win = board.check_win()
 if win or board.is_full():
 board.display()
 if win:
 print(f"player {player.name} won")
 player.score += 1
 print(f"current scores:\nPlayer {players[0].name}: {players[0].score}")
 print(f"Player {players[1].name}: {players[1].score}")
 elif board.is_full():
 print("It's a draw!")
 again = input("another game?(y/n)")
 if again == "y":
 board = Board()
 continue
 return
if __name__ == '__main__':
 main()

• hjpotter92: Is this python 3.9+? or plan to go for 3.9+?
• Nadi726: It's in python 3.8.5, and I don't plan to update it to python 3.9.

Answer 1

Welcome to Code Review!

1. Having a class for player seems over complicated. A simple namedtuple would be enough.

2. The main() function is doing most of the heavy lifting. You can have a Game class, which takes the players list (or individual objects) as init parameters, and then implements the game logic.

3. You can reuse the get_player_input when asking for another game from the user.

4. When working with random library, it is generally a good practice to seed it at the beginning.

5. The Board.display cam be made a 1-liner:

    print("\n".join(" ".join(row) for row in self._board))
   

6. Instead of having a Board.display method, override __str__, and simply print(board).

7. Alternative implementation of is_full:

    def is_full(self):
    return "-" not in set(chain(*self._board))
   

   where chain is from itertools.

• Nadi726: You have some great points, thanks! However, there's a problem with your 5th one: first, I assume you meant row instead of x. Second, it raises an error once there's a player object on the row. I managed to fix it by modifying it to '\n'.join(' '.join([str(tile) for tile in row]) for row in self._board) and overriding str for Player
• Nadi726: Also... I'm not sure I understand the 2nd one completely. if the problem is main() doing most of the heavy lifting, wouldn't it be simpler to just have a game function instead of class?
• hjpotter92: @Nadi726 yes. it was supposed to be row. edited now. There is not an issue with having the whole functionality in main(); however, if you plan to have multiple games running in parallel (for stats/multiplayer for eg.) it would become hectic to run the program n-different times. With a Game class, you can maintain multiple independent copies, scores, player information etc. as its own entity. Later, you may for eg. store game results in a centralised db. Just a suggestion for futuristic expansion. Not needed if the console level is its intended use.
• Nadi726: Even with row instead of x, your code still fails if there's a Player object in the row, as I mentioned in my comment - you can try to run it yourself. And I think I can see your point about main, although I'm not sure about how to implement the Game class without rewriting a lot of the code, but I'll try.
• hjpotter92: @Nadi726 Board should not be dealing with player objects at all. When placing a symbol, you only place the symbol. The Game manager will handle which player's symbol was it. Players can also choose their own symbols if needed.

Answer 2

Welcome to CodeReview!

You have missed an OO-opportunity.

You have a class Player but you are still "switching on internal data". You do this:

def get_player_input(choices, text=''):
 ...

which is not a method on Player. And later, you do this:

if player.is_human:
 row = int(get_player_input(options, "Enter row number(1-3): ")) - 1
 colmn = int(get_player_input(options, "Enter column number(1-3): ")) - 1
else:
 row, colmn = randint(0, 2), randint(0, 2)

This act of writing if player.is_human: ... else: ... is "switching on internal data". It's "internal data" because you aren't getting it from outside the class. It's "switching" because you are making an exclusive choice.

Switching on internal data is a "code smell" that indicates you might need a new class. In this case, I think you do:

from abc import ABC, abstractmethod
class Player(ABC):
 @abstractmethod
 def get_next_move(self, board: Board) -> Position:
 ...
class PlayerIO(Player):
 def get_next_move(self, board: Board) -> Position:
 """ Read next move from io streams """
 pass
class PlayerRandom(Player):
 def get_next_move(self, board: Board) -> Position:
 """ Randomly generate next move """
 pass

I'll suggest that the IO constructor takes input and output streams, and handles displaying the board and prompting for a new move.

I'll also suggest that you write a TextIO class of some kind, and give it methods like "prompt for input" and "read a string" and "read an integer". This basic set of operations can be the building blocks for your PlayerIO class, and will make it possible to create a mock object for unit testing.

Answer 3

In check_win:

checks = {True if (len(set(lst)) == 1 and lst[0] != "-") else False for lst in checks}
if True in checks:
 return True
return False

can be rewritten more explicitly as:

return any(len(set(lst)) == 1 and lst[0] != "-" for lst in checks)

• Peilonrayz: This also has the benefit that any is lazy so it'll stop checking once finding the first True.

Answer 4

This looks good and well thought through.

The continue, break and return statements are always a bit tricky, it might be useful to add a comment to them to explain what they continue/break/return from.

Theoretically we could be in an endless loop if the computer player never finds a non-full tile.... , but the AI algorithm you planned will fix that :-)