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()

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.

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)

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 :-)