Beginner Tic Tac Toe Game for Python

Question

I am a beginner programmer and would like some feedback on this Tic Tac Toe game I made on Python. Any feedback would be appreciated, as I really want to get better at programming and pick up good habits. Here is my code. Something I would add is try statements for invalid user input, but this is not important as I would like feedback on how to optimise this code. I would also love feedback on the check_if_win() function as I believe there might be a better way of doing this. Thank you!

"""Tic Tac Toe"""

__author__ = "Enrique Gonzalez Aretos"

EMPTY = " "
PLAYER_1 = "X"
PLAYER_2 = "O"

# Create Board
def create_board():
    """
    Function which creates an empty game board
    :return: <list<list<str>>> Returns the game board
    """
    return [
        [EMPTY, EMPTY, EMPTY],
        [EMPTY, EMPTY, EMPTY],
        [EMPTY, EMPTY, EMPTY]
    ]
# Show board
def show_board(board):
    """
    Function which prints the board for the user
    :param board: <list<list<str>>> The Tic Tac Toe board
    :return: <str> A clean-looking version of the board
    """
    for x in board:
        # Find better way to print this
        print("|".join(x))

# Place X on spot
def place_player(board, active_player, row, column):
    """
    Function which places the player into the board
    :param board:
    :param active_player:
    :param row:
    :param column:
    :return:
    """
    if board[row-1][column-1] == EMPTY:
        board[row-1][column-1] = active_player

# Check if someone has won
def check_if_win(board, active_player):
    """
    Function which checks if the active player has won
    :param board: <list<list<str>>> Tic Tac Toe Board
    :param active_player: <str> The current active player
    :return: <bool> Return True or False
    """
    # Making lists which will store the diagonals and columns of baord
    diagonal1 = []
    diagonal2 = []
    column1 = []
    column2 = []
    column3 = []
    # Diagonal counter is used to index the different values in each row
    diagonal_counter = 1
    for row in board:
        # When each row is checked, the different indexes are organised
        # into the column and diagonal lists
        column1.append(row[0])
        column2.append(row[1])
        column3.append(row[2])
        diagonal1.append(row[diagonal_counter-1])
        diagonal2.append(row[-diagonal_counter])
        diagonal_counter += 1
        if "".join(row) == active_player * 3:
            return True

    # If statement which checks if any list is full of the active 
    # player's symbol
    if "".join(column1) == active_player * 3 or "".join(column2) == 
    active_player * 3 or "".join(column3) == active_player * 3:
        return True
    elif "".join(diagonal1) == active_player * 3 or "".join(diagonal2) 
    == active_player * 3:
        return True
    else:
        return False

def swap_player(active_player):
    if active_player == PLAYER_1:
        return PLAYER_2
    else:
        return PLAYER_1


def main():
    # Creates board and assigns starting player
    board = create_board()
    active_player = PLAYER_1
    while True:
        show_board(board)
        # Ask for player input
        row = int(input("\nIt is {}'s turn play. Choose a row (1-3): 
              ".format(active_player)))
        column = int(input("Choose a column: "))
        place_player(board, active_player, row, column)
        # Checks if player has won
        if check_if_win(board, active_player) is True:
            show_board(board)
            print("\n{} won!".format(active_player))
            break
        active_player = swap_player(active_player)

if __name__ == "__main__":
    main()

Answer 1

create_board()

You explicitly return a hard-coded board, using 5 lines of code. This could be written as one line, using list multiplication and list comprehension:

return [ [EMPTY]*3 for _ in range(3) ]

[EMPTY]*3 creates a list of 3 EMPTY values: [EMPTY, EMPTY, EMPTY], and the outer [ ... for _ in range(3) ] repeats that operation 3 times, producing the required board. (As pointed out in the comments, using list multiplication for the outer loop doesn’t produce 3 unique rows, but 3 references to the same row.)

show_board()

This function does not return anything, so certainly does not :return: <str> A clean-looking version of the board

place_player()

Despite the comment above the function, this method can be used to place an "O" on a spot on the board.

Suggestion: indicate somehow if the move is invalid (beyond row 1..3 or column 1..3, or if the spot is not empty, such as by raising an exception or returning a success/failure code.

check_if_win()

This function is using active_player * 3 in a total of six places. Instead of repeating this expression six times, you could assign the result to a local variable, and test against that variable instead:

win_pattern = active_player * 3
# ...
for row in board:
   # ...
   if "".join(row) == win_pattern:
      return True
# ... etc ...

But a winning pattern isn't actually a string; it is a list of cells each containing the active_player symbol: ['X', 'X', 'X']. You've just used "".join(...) to convert the list into a string. Why create the string when you can just check for equality with the list?

win_pattern = [ active_player ] * 3
# ...
for row in board:
   # ...
   if row == win_pattern:
       return True

if column1 == win_pattern or column2 == win_pattern or column3 == win_pattern:
    return True
elif diagonal1 == win_pattern or diagonal2 == win_pattern:
    return True
else:
    return False

But this code is still quite verbose. There is a pythonic way of testing if any item in a list of items matches a condition. It uses the any(iterable) call; if any of the iterable items is True, the resulting any(...) expression evaluates to True. Using this to test for any of the rows of the board matching the win_pattern is straight forward:

if any(row == win_pattern for row in board):
    return True

Your for row in board: loop also constructs the column1, column2, column3, diagonal1 and diagonal2 lists. Whenever you have a set of variables with the same prefix name and numeric suffixes, you aught to ask yourself if you could use a list (for example, column[0], column[1], and column[2]) or a loop. Let's start by making a single column in a loop:

column = []
for row in board:
    column.append(row[col])

If col was 0, this would make your column1 variable from your code. List comprehension can reduce this to one line:

column = [ row[col] for row in board ]

We actually only need the column variable for just the test against win_pattern, so we could actually eliminate it and perform the test directly:

if [ row[col] for row in board ] == win_pattern:
   return True

Doing that in a loop, for all 3 col values completes the test for a win in any column:

for col in range(3):
    if [ row[col] for row in board ] == win_pattern:
       return True

But above, we replaced a for x in y: and an inner if condition: with an any(...) statement; we can do the same thing here!

if any( [ row[col] for row in board ] == win_pattern for col in range(3)):
    return True

We can also do this for the forward diagonal:

if [ board[i][i] for i in range(3) ] == win_pattern:
    return True

And the reverse diagonal:

if [ board[i][2-i] for i in range(3) ] == win_pattern:
    return True

This looks a lot shorter, and simpler:

win_pattern = [ active_player ] * 3

if any(row == win_pattern for row in board):
    return True

if any( [ row[col] for row in board ] == win_pattern for col in range(3)):
    return True

if [ board[i][i] for i in range(3) ] == win_pattern:
    return True

if [ board[i][2-i] for i in range(3) ] == win_pattern:
    return True

return False

... but I don't quite like it yet. We can get rid of the win_pattern list, and all the [ ... for x in y ] list creation, using something similar to any(...): the all(...) function. It returns True when all elements are True. So instead of

if [ row[col] for row in board ] == [ active_player ] * 3:

which creates a list of 3 items on the left-hand-side and creates a list of 3 items on the right hand side and tests if the lists are the same, we'll test if the first element on the left-hand-side is the same as the first element on the right-hand-side, and ditto for the second elements, and ditto for the third elements. Of course, all 3 elements on the right-hand-side are the same element. So the all(...) statement will look like:

if all(row[col] == active_player for row in board):

Again, that is testing one particular column index: col. If we want to check if any column has the winning pattern, we'll wrap this in an any call:

if any( all(row[col] == active_player for row in board) for col in range(3)):
    return True

So, changing the implementation slightly, to exploit the duality nature of rows and columns, you might write this method as:

def check_if_win(board, active_player):

    if any( all(board[r][c] == active_player for c in range(3)) for r in range(3)):
        return True

    if any( all(board[r][c] == active_player for r in range(3)) for c in range(3)):
        return True

    if all(board[i][i] == active_player for i in range(3)):
        return True

    if all(board[i][2-i] == active_player for i in range(3)):
        return True

    return False

main()

if check_if_win(board, active_player) is True:

could be written without the is True portion:

if check_if_win(board, active_player):

Asking for user input, and converting it to an integer int(input(...)) can lead to ValueError if the user enter bad values, such as "fish". You should check for and catch exceptions. Moreover, you should range-check the input. A user could enter 42 -1 for the row/column. Finally, the user could enter a valid, legal board position which is already used. If the user enters bad values (either not valid integers, or out-of-bound values, or already taken locations), the program should complain and ask the user to re-enter the row column input.

Bug: Finally, the game could end in a Tie, which is currently not handled.