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.

• Wombatz: [[EMPTY] * 3] * 3 does not create copies of the inner list. After board[0][0] = PLAYER_1 you will end up with [['X', ' ', ' '], ['X', ' ', ' '], ['X', ' ', ' ']].
• AJNeufeld: @Wombatz D’oh! I knew that. I should know better than adding a last second addition to the code review post, especially to the very start of the post no less!