10
\$\begingroup\$

I am a new programmer, learning from youtube and other sites I can get my hands on. The assignment was clear. Create a Tic Tac Toe game using functions as much as possible and where needed.

I am posting this because I am very much interested what this community thinks of this effort and also if there would be any suggestions.

The code is a 2 player game. No AI.

This is what the code does:

  • Display welcome and instructions
  • Display board
  • We start with player X
  • While no one has won and it isn't a tie loop
  • get player's move
  • Update the board with the new move and check legal move, so move that hasn't been taken
  • Switch Turns
  • Congratulate Winner or say it's a Tie

Actual code:

def show_welcome_message():
    """Display Welcome to Tic Tac Toe """
    print(
        """
        Hi dear players and welcome to this Tic Tac Toe game_board.
        You can win by having three 'X' or three 'O' in a row.
        This can be horizontal, vertical or diagonal
    
       """
    )

def show_game_board(game_board):
    """display the board"""
    for x in range(3):
        print("+---+---+---+" )
        for y in range(3):
            print("|", game_board[x*3+y], end = " ")
        print("|")
    print("+---+---+---+" )


def check_winner(game_board):
    """check if there is a win, draw"""
    # check rows
    for i in range(3):
        if game_board[i*3] == game_board[i*3+1] == game_board[i*3+2] and game_board[i*3] in ('X', 'O'):
            return game_board[i*3]
    # check columns
    for i in range(3):
        if game_board[i] == game_board[i+3] == game_board[i+6] and game_board[i] in ('X', 'O'):
            return game_board[i]
    # check diagonals
    if game_board[0] == game_board[4] == game_board[8] and game_board[0] in ('X', 'O'):
        return game_board[0]
    if game_board[2] == game_board[4] == game_board[6] and game_board[2] in ('X', 'O'):
        return game_board[2]
    # check tie
    if all(x in ('X', 'O') for x in game_board):
        return 'TIE'  #lower case
    # no winner or tie
    return False

def get_player_move(game_board,user_piece):
    """ Get the player's move """
    player_move = int(input(f" Please make a move on an unoccupied square player with piece {user_piece} "))
    while not is_move_legal(game_board,player_move):
        player_move = int(input( " That move is not legal try again "))
    return player_move

def is_move_legal(game_board,player_move):
    """Check if the move is legal"""
    legal_play = True
    if game_board[player_move -1] in ['X','O']:
        legal_play = False
    return legal_play
    
    
def update_game_board(game_board,user_piece,player_move):
    """ Modify the board positions into X or O"""
    game_board[player_move - 1] = user_piece
    

def display_game_result(user_piece):
    """ display win for X or O or a tie"""
    if user_piece == "TIE":
        print("This is a TIE players")
    else:
        print(f"Yes the player with {user_piece} won !!" )

#Addition of stackoverflow
def change_player_turns(user_piece):
    """ Change player's piece into X or O"""
    return 'O' if user_piece == 'X' else 'X'

def main():
    """ game_board starting point """
    game_board = [1,2,3,4,5,6,7,8,9]
    user_piece = None
    show_welcome_message()
    show_game_board(game_board)
    user_piece = 'X'
    print(f"First player starts with {user_piece}")

    while not check_winner(game_board):
        player_move = get_player_move(game_board,user_piece)
        update_game_board(game_board,user_piece,player_move)
        show_game_board(game_board)
        user_piece = change_player_turns(user_piece)

    display_game_result(check_winner(game_board))    

if __name__ == "__main__":
    main()
\$\endgroup\$
4
  • 7
    \$\begingroup\$ A main guard and no globals! Impressive for a beginner. \$\endgroup\$
    – Reinderien
    Commented Aug 24, 2023 at 13:50
  • \$\begingroup\$ Thank you so much @Reinderien ! \$\endgroup\$ Commented Aug 24, 2023 at 14:55
  • \$\begingroup\$ You do not need the Here is \$\endgroup\$
    – user275173
    Commented Aug 25, 2023 at 0:04
  • 5
    \$\begingroup\$ @KimiM We sometimes run into title overlap. There are a lot of Tic Tac Toe games written in python. \$\endgroup\$
    – pacmaninbw
    Commented Aug 25, 2023 at 0:33

3 Answers 3

6
\$\begingroup\$

I would say you did an excellent job overall! The only suggestions I would make are:

The User Is Capable of Entering Unexpected Input

You have:

player_move = int(input( " That move is not legal try again "))

What if the user enters the letter a? This will prematurely terminate your game with a ValueError exception.

You should catch this exception. Even if an integer is entered, is it in the range 1..9? If not it could raise an IndexError when the game board is accessed (e.g. if '10' were entered). The user could also enter '-1', which will not raise an IndexError, but I don't think you would want to accept this either. So I would explicitly check that the entered value is within the acceptable range.


Avoid Re-calculating the Same Value

For example, you have in function main:

    while not check_winner(game_board):
        player_move = get_player_move(game_board,user_piece)
        update_game_board(game_board,user_piece,player_move)
        show_game_board(game_board)
        user_piece = change_player_turns(user_piece)

    display_game_result(check_winner(game_board))    

After you break out of the while loop you will make a second call to check_winner with the same game_board argument value. You can avoid this redundant and relatively expensive call by using the so-called walrus assignment operator (:=):

    while not (winner := check_winner(game_board)):
        player_move = get_player_move(game_board,user_piece)
        update_game_board(game_board,user_piece,player_move)
        show_game_board(game_board)
        user_piece = change_player_turns(user_piece)

    display_game_result(winner)

If you do not want to use the walrus assignment operator, then consider:

    while True:
        winner = check_winner(game_board)
        if winner:
            break
        player_move = get_player_move(game_board,user_piece)
        update_game_board(game_board,user_piece,player_move)
        show_game_board(game_board)
        user_piece = change_player_turns(user_piece)

    display_game_result(winner)

Remove Extraneous Spaces From Output

I hesitated to include this section since it is highly subjective and it doesn't go to the heart of what you are trying to accomplish. But for what it's worth:

Your introduction to the players beginning with Hi dear players... is indented 8 spaces. This is strictly a matter of taste but I don't see that indentation as adding anything. It's not as if you were working with a fixed-width output device and you were attempting to center the text (that would be a different story).

Also, when you solicit a move I see:

+---+---+---+
| 1 | 2 | 3 |
+---+---+---+
| 4 | 5 | 6 |
+---+---+---+
| 7 | 8 | 9 |
+---+---+---+
First player starts with X
 Please make a move on an unoccupied square player with piece X

That single-space indentation on the last line should be removed. Perhaps the instruction should read:

Player X, please make a move by specifying the number of an unoccupied square:

Finally:

Bravo! (or should I say: Brava!)

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thank you so much @Booboo . I will implement these. Haha yes Brava is perfect ! Muchas gracias! \$\endgroup\$ Commented Aug 25, 2023 at 8:48
3
\$\begingroup\$

Excellent question. I have several suggestions.

  • Check indices. game_board[-1] is accepted, but you probably don't intend 0 to be a move.
  • To be more robust, use try/except to catch errors from int().
  • Use type hints to help with parameter validation for functions.
  • Define functions like is_move_legal before you use them for readability.
  • Add parameter information to your docstrings for readability.

If you wish to extend the project further, here are some ideas:

  • What if the number of rows / columns was flexible?
  • What if the number of marks-in-a-row to win was flexible?
  • What if the number of players was flexible?
  • What if the board could have more than 2 dimensions?
  • What if the user wanted to play against bot(s)?

I have implemented the suggestions and the first 3 ideas below. Separating features into classes like GameBoard can make code more modular:

class GameBoard:
    def __init__(self, m: int, n: int, win: int):
        """
        Creates a board for generalized TicTacToe.

        :param m: number of rows
        :param n: number of cols
        :param win: number of consecutive marks to win
        """
        if m < 1:
            raise ValueError("Number of rows must be positive!")
        if n < 1:
            raise ValueError("Number of cols must be positive!")
        if win < 1:
            raise ValueError("Marks per win must be positive!")
        self.m = m
        self.n = n
        self.win = win
        # board[i][j] is None if empty or a player's mark
        self.board = [[None] * n for _ in range(m)]

    def __str__(self):
        """
        Returns this object in string form.

        :return: a regular grid of entries
        """
        m = self.m
        n = self.n

        # max number of characters per entry
        max_width = len(str(m * n))
        row_div = "+"  # divider per row
        for j in range(n):
            row_div += ("-" * max_width + "--+")

        # build the board as a string
        result = row_div
        for i in range(m):
            result += "\n"
            for j in range(n):
                value = self.board[i][j]
                if value is None:
                    value = i * n + j + 1
                display = str(value)
                blanks = " " * (max_width - len(display))
                result += f"| {display}{blanks} "
            result += "|\n" + row_div
        return result

    def move_worked(self, move_input: str, player: str):
        """
        Attempt a move and return whether it worked.

        :param move_input: the player's input
        :param player: the player's mark (e.g. X, O)
        :return: whether the move was valid and played
        """
        try:
            move = int(move_input) - 1
        except ValueError:
            return False

        m = self.m
        n = self.n
        i = move // n
        j = move % n

        if 0 <= i < m and 0 <= j < n and self.board[i][j] is None:
            self.board[i][j] = player
            return True
        return False

    def get_winner(self):
        """
        Returns the first winner found (or None if no winners).

        :return: string or None
        """
        m = self.m
        n = self.n
        win = self.win
        board = self.board

        for i in range(m):
            for j in range(n):
                # leftmost top mark of the pattern
                mark = board[i][j]
                if mark is None:
                    continue

                if j + win <= n:
                    # row
                    if all(board[i][j + k] == mark for k in range(win)):
                        return mark
                if i + win <= m:
                    # column
                    if all(board[i + k][j] == mark for k in range(win)):
                        return mark
                    # top-left to bottom-right diagonal
                    if j + win <= n:
                        if all(board[i + k][j + k] == mark for k in range(win)):
                            return mark
                    # top-right to bottom-left diagonal
                    if j + 1 >= win:
                        if all(board[i + k][j - k] == mark for k in range(win)):
                            return mark

        # no winner yet
        return None


def display_game_result(winner):
    """
    Displays the outcome of a game

    :param winner: string representing player's mark
    """
    if winner is None:
        print("It's a tie!")
    else:
        print(f"Player {winner} won!!!")


def main():
    m = 7
    n = 5
    win = 5
    user_pieces = ['X', 'O', 'Y']

    print(f"""
Welcome to Tic Tac Toe! Our board is {m} x {n}.
You can win by having {win} marks in a line.
Lines can be horizontal, vertical or diagonal.
""")

    game_board = GameBoard(m, n, win)
    turn_count = 0
    # after m * n moves without a winner, it's a tie
    while game_board.get_winner() is None and turn_count < m * n:
        print(game_board)
        user_piece = user_pieces[turn_count % len(user_pieces)]
        turn_count += 1
        print(f"Turn {turn_count}: {user_piece} to play ...")
        player_move = input("Please enter the number of the square to mark: ")
        while not game_board.move_worked(player_move, user_piece):
            player_move = input("That move is not legal, try again. ")
    print(game_board)
    display_game_result(game_board.get_winner())


if __name__ == "__main__":
    main()

Keep up the great work!

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thank you very much @Justin Change !! \$\endgroup\$ Commented Aug 25, 2023 at 12:55
1
\$\begingroup\$

Missing input validation

The user can input any text, only some of them will be successfully converted to int, and even fewer is valid (x in range(1, 10)). What if the user inputs cat, what happens then? A ValueError exception is thrown, because 'cat' cannot be converted to an integer (unless you are converting a numeral from a base no less than 30, cat30 = 1112910, in this case you need to specify base parameter in int call: int('cat', 30) == 11129):

In [76]: int('cat')
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[76], line 1
----> 1 int('cat')

ValueError: invalid literal for int() with base 10: 'cat'

And for numbers not in range(1, 10), your code will cause IndexError:

First player starts with X
 Please make a move on an unoccupied square player with piece X 11
IndexError: list index out of range

I have refactored your function to get user input and added input validation:

import contextlib

def get_player_move(game_board, user_piece):
    """ Get the player's move """
    while True:
        choice = -1
        while not 0 < choice < 10:
            while True:
                with contextlib.suppress(ValueError):
                    choice = int(input(f" Please make a move on an unoccupied square player with piece {user_piece} "))
                    break
        if game_board[choice -1] not in {'X','O'}:
            break
    return choice

Complicated code to check winner

Your code to check winner is very complicated, and much more complicated than it should be.

There is a lot of repetition in the code, the conditions to check for columns, rows and diagonals are almost identical, except for the indices.

(0, 1, 2), (3, 4, 5), (6, 7, 8) indices are on the same row, (0, 3, 6), (1, 4, 7), (2, 5, 8) indices are on the same column. (0, 4, 8), (2, 4, 6) are the diagonals.

How to get values at those indices? Notice in each group, the difference between two consecutive indices are the same? For indices in the same row, the step is 1, in the same column the step is 3, then for one diagonal the step is 4 for the other diagonal the step is 2.

In Python we use range to generate numbers, range(start, stop, step) will generate the following numbers: start, start + step, start + 2 * step ... (stop), the stop value is not included. For example, list(range(0, 3, 1)) == [0, 1, 2].

We can use slicing to get values located at those indices, for example, list(range(1, 10)[0:9:4]) == [1, 5, 9].

We can store the start and stop indices and the steps in a nested tuple and just then iterate through it to check all conditions.

LINES = (
    (0, 3, 1),
    (3, 6, 1),
    (6, 9, 1),
    (0, 7, 3),
    (1, 8, 3),
    (2, 9, 3),
    (0, 9, 4),
    (2, 7, 2)
)

Now how do we check if all elements are equal? In Python, elements of a set are unique, so if len(set(collection)) == 1, then all elements in collection are the same.

def check_winner(game_board):
    """check if there is a win, draw"""
    for start, stop, step in LINES:
        if len(set(game_board[start:stop:step])) == 1 and (winner := game_board[start]) in {'X', 'O'}:
            return winner
    return 'TIE' if all(x in ('X', 'O') for x in game_board) else False
\$\endgroup\$
2
  • \$\begingroup\$ Thank you so much! It gives me a new insight in how to achieve this \$\endgroup\$ Commented Aug 25, 2023 at 13:05
  • \$\begingroup\$ I wouldn't say it's good advice for a beginner to merge small functions into bigger ones. A codebase with (sensible) small functions is often easier to read and definitely easier to unit-test. OP struck a decent balance here. I like your rewrite of the win condition though. \$\endgroup\$
    – Seb
    Commented Aug 25, 2023 at 15:41

Not the answer you're looking for? Browse other questions tagged or ask your own question.