Python: Tic Tac Toe Game

Question

I am here to ask for some more experienced programmers to critique my first project. I wanted to see where I lack and what I can improve on. Thank you for taking time out of your day to help me better my skills as a programmer! I hope I did a good job for being a beginner but I know there is much more to learn!

game_board = ['1','2','3','4','5','6','7','8','9']
original_board = ['1','2','3','4','5','6','7','8','9']
player1 = ''
player2 = ''
isFull = False
game_on = True
winner = False
def display_board(board):
 print(board[0]+'|'+board[1]+'|'+board[2])
 print(board[3]+'|'+board[4]+'|'+board[5])
 print(board[6]+'|'+board[7]+'|'+board[8])
def player_input():
 global player1
 global player2
 while player1 != 'X' or player1 != 'O':
 player1 = input("Which do you want to be? (X or O): ")
 player1 = player1.upper()
 if player1 == 'X':
 print("Player 1 has chosen X.")
 player1 = 'X'
 player2 = 'O'
 break
 elif player1 == 'O':
 print("Player 1 has chosen O.")
 player1 = 'O'
 player2 = 'X'
 break
 return player1
def place_marker(board, player, position):
 board[position-1] = player
 
 return board[position-1]
def win_check(board):
 global player1
 global player2
 global game_on
 global winner
 if board[0] == 'O' and board[1] == 'O' and board[2] == 'O':
 
 if player1 == 'O':
 print("Player 1 (O) has won!")
 game_on = False
 winner = True
 else:
 print("Player 2 (O) has won!")
 game_on = False
 winner = True
 elif board[0] == 'O' and board[4] == 'O' and board[8] == 'O':
 
 if player1 == 'O':
 print("Player 1 (O) has won!")
 game_on = False
 winner = True
 else:
 print("Player 2 (O) has won!")
 game_on = False
 winner = True
 elif board[2] == 'O' and board[4] == 'O' and board[6] == 'O':
 if player1 == 'O':
 print("Player 1 (O) has won!")
 game_on = False
 winner = True
 else:
 print("Player 2 (O) has won!")
 game_on = False
 winner = True
 elif board[0] == 'O' and board[3] == 'O' and board[6] == 'O':
 if player1 == 'O':
 print("Player 1 (O) has won!")
 game_on = False
 winner = True
 else:
 print("Player 2 (O) has won!")
 game_on = False
 winner = True
 elif board[1] == 'O' and board[4] == 'O' and board[7] == 'O':
 if player1 == 'O':
 print("Player 1 (O) has won!")
 game_on = False
 winner = True
 else:
 print("Player 2 (O) has won!")
 game_on = False
 winner = True
 elif board[2] == 'O' and board[5] == 'O' and board[8] == 'O':
 if player1 == 'O':
 print("Player 1 (O) has won!")
 game_on = False
 winner = True
 else:
 print("Player 2 (O) has won!")
 game_on = False
 winner = True
 elif board[3] == 'O' and board[4] == 'O' and board[5] == 'O':
 if player1 == 'O':
 print("Player 1 (O) has won!")
 game_on = False
 winner = True
 else:
 print("Player 2 (O) has won!")
 game_on = False
 winner = True
 elif board[6] == 'O' and board[7] == 'O' and board[8] == 'O':
 if player1 == 'O':
 print("Player 1 (O) has won!")
 game_on = False
 winner = True
 else:
 print("Player 2 (O) has won!")
 game_on = False
 winner = True
 
 if board[0] == 'X' and board[1] == 'X' and board[2] == 'X':
 
 if player1 == 'X':
 print("Player 1 (X) has won!")
 game_on = False
 winner = True
 else:
 print("Player 2 (X) has won!")
 game_on = False
 winner = True
 elif board[0] == 'X' and board[4] == 'X' and board[8] == 'X':
 
 if player1 == 'X':
 print("Player 1 (X) has won!")
 game_on = False
 winner = True
 else:
 print("Player 2 (X) has won!")
 game_on = False
 winner = True
 elif board[2] == 'X' and board[4] == 'X' and board[6] == 'X':
 if player1 == 'X':
 print("Player 1 (X) has won!")
 game_on = False
 winner = True
 else:
 print("Player 2 (X) has won!")
 game_on = False
 winner = True
 elif board[0] == 'X' and board[3] == 'X' and board[6] == 'X':
 if player1 == 'X':
 print("Player 1 (X) has won!")
 game_on = False
 winner = True
 else:
 print("Player 2 (X) has won!")
 game_on = False
 winner = True
 elif board[1] == 'X' and board[4] == 'X' and board[7] == 'X':
 if player1 == 'X':
 print("Player 1 (X) has won!")
 game_on = False
 winner = True
 else:
 print("Player 2 (X) has won!")
 game_on = False
 winner = True
 elif board[2] == 'X' and board[5] == 'X' and board[8] == 'X':
 if player1 == 'X':
 print("Player 1 (X) has won!")
 game_on = False
 winner = True
 else:
 print("Player 2 (X) has won!")
 game_on = False
 winner = True
 elif board[3] == 'X' and board[4] == 'X' and board[5] == 'X':
 if player1 == 'X':
 print("Player 1 (X) has won!")
 game_on = False
 winner = True
 else:
 print("Player 2 (X) has won!")
 game_on = False
 winner = True
 elif board[6] == 'X' and board[7] == 'X' and board[8] == 'X':
 if player1 == 'X':
 print("Player 1 (X) has won!")
 game_on = False
 winner = True
 else:
 print("Player 2 (X) has won!")
 game_on = False
 winner = True
def space_check(board, position):
 isTaken = False
 if board[position - 1] == 'X' or board[position - 1] == 'O':
 print("That position is already taken choose again.")
 isTaken = True
 else:
 pass
 return isTaken
 
def full_board_check(board):
 global isFull
 global original_board
 for num in range(9):
 if board[num] == original_board[num]:
 return isFull
 else:
 isFull = True
 return isFull
def player_choice(board):
 good = False
 while good == False:
 choice = int(input("Choose a spot: "))
 space_check(board, choice)
 if space_check(board, choice) == False:
 good = True
 return choice
 return choice
def replay():
 global game_board
 global player1
 global player2
 global isFull
 global game_on
 
 keep_playing = True
 while keep_playing == True:
 replay = input("Do you want to replay? (Y or N): ")
 replay = replay.upper()
 if replay == 'Y':
 game_board = ['1','2','3','4','5','6','7','8','9']
 player1 = ''
 player2 = ''
 game_on = True
 isFull = False
 elif replay == 'N':
 keep_playing = False
 print("Thanks for playing!")
print("Welcome to Tic Tac Toe!")
while isFull == False:
 display_board(game_board)
 player_input()
 while game_on:
 
 full_board_check(game_board)
 if isFull == True and winner == False:
 print("The game is a tie!")
 game_on = False
 break
 if game_on:
 print("Player 1's turn.")
 place_marker(game_board, player1, player_choice(game_board))
 display_board(game_board)
 win_check(game_board)
 full_board_check(game_board)
 if isFull == True and winner == False:
 print("The game is a tie!")
 game_on = False
 break
 if game_on:
 print("Player 2's turn.")
 place_marker(game_board, player2, player_choice(game_board))
 display_board(game_board)
 win_check(game_board)
 
 break

Answer 1

I'll point out some useful starting points for improving your code. I would additionally recommend looking at some other implementations, for some further inspiration regarding logic and code structure. There are a lot on this site alone, here's a starting point from recent memory: Tic-tac-toe in python with OOP.

---

Code structure

You rely on a few global variables (game_board, original_board, player1, player2, isFull, game_on, winner). This is generally not considered good practice for a variety of reasons and it can be avoided in most cases.

---

Naming

PEP 8: "Function names should be lowercase, with words separated by underscores as necessary to improve readability. Variable names follow the same convention as function names."

---

Unnecessary return

Most of your functions return a value, but since they change the global variables the return value is never needed / used (e.g. player_input, place_marker, full_board_check). In your current implementation you could simply delete those return statements as they're never used. In an improved version I recommend using the return values instead of writing to global variables.

---

display_board

Very rarely in Python (and programming in general) should you hardcode indices as it makes code less readable, maintainable and scalable. Consider this approach using list slicing:

def display_board(board):
 row_length = 3
 for i in range(0, len(board), row_length):
 print('|'.join(board[i:i+row_length]))

Now if you ever want to change the board size / row length or the column separator, it will be a lot easier.

---

player_input

Consider this improved function:

def player_input():
 global player1
 global player2
 options = {'X', 'O'}
 while True:
 player1 = input(f"Which do you want to be? ({' or '.join(options)}): ").upper()
 if player1 in options:
 options.remove(player1)
 player2 = options.pop()
 break
 print(f"Player 1 has chosen {player1}.")
 print(f"Player 2 is {player2}.")

A few things to notice here:

• Your while loop had redundant logic: It checks if player1 is not in {'X', 'O'} each iteration, but once player1 is either of those values it immediately breaks. Meaning that the while-condition will never be relevant.
• The assignment if player1 == 'X': ... player1 = 'X' is redundant.
• We can pull out the print statement to the end of the function, as the text is the exact same, except for the value of player1. f-Strings provide a really convenient and redable way to include variables in a string.
• If we decide to change player symbols, this function only requires changes in a single place. The options for player symbols should probably be moved to a module constant to make changing them even easier.

---

win_check

This is probably the function you could and should save yourself the most work on. Manually writing out all the possible winning configurations is error-prone and hard to maintain. I'd recommend looking at some other implementations (and the corresponding reviews) to get an idea of how to make this easier for yourself.

Everytime you find yourself copying or repeating code, you should stop to think if there's a pattern there. A naive implementation is often a good starting point to familiarize yourself with the problem. Once you have an understanding for the underlying logic you should start looking to improve your approach. For example: Even if you still manually check all the possible winning configurations, you should pull out the common part of the different conditions into a separate function. I'm talking about this part, which repeats 10+ times in this single function:

if player1 == 'O':
 print("Player 1 (O) has won!")
 game_on = False
 winner = True
else:
 print("Player 2 (O) has won!")
 game_on = False
 winner = True

This code snippet should also be simplified to something like

winner_number = 1 if player1 == 'O' else 2
print(f"Player {winner_number} (O) has won!")
game_on = False
winner = True

Another consideration for further improvements:

These two conditions

if board[0] == 'O' and board[1] == 'O' and board[2] == 'O'
if board[0] == 'X' and board[1] == 'X' and board[2] == 'X'

use the same logic. The only variable is the symbol that's checked. This should be an indication that a variable should be used instead of all the possible values. We only need to check if the spots contain the same symbol, not which one exactly it is:

if board[0] == board[1] == board[2]:
 winning_symbol = board[0]
 winning_number = 1 if player1 == winning_symbol else 2
 print(f"Player {winning_number} ({winning_symbol}) has won!")
 game_on = False
 winner = True

Again, the code inside the condition should be put into its own function to reduce code duplication.

---

These suggestions are merely starting points for improvements, there are a lot more simplifications that can be done. You're always welcome to come back to CodeReview with an improved version to get another round of feedback. =)

• KeyesCode: Thank you so much for the review! I am currently going through your suggestions and will keep them in mind for the next project I work on. Thank you again for taking time out of your day to help a newbie like myself.

Answer 2

You need to add exception handling in player_choice. Currently, if I type in 'Y', the game crashes with error code:

Traceback (most recent call last):
 File "/Users/jake/Documents/TTRreview.py", line 299, in <module>
 place_marker(game_board, player2, player_choice(game_board))
 File "/Users/jake/Documents/TTRreview.py", line 246, in player_choice
 choice = int(input("Choose a spot: "))
ValueError: invalid literal for int() with base 10: 'y'

also, if I type an integer larger that 9 or smaller than one, the game behaves oddly, because python simply rolls over the array instead of giving an error. Change the code to read:

def player_choice(board):
 good = False
 while good == False:
 try:
 choice = int(input("Choose a spot: ")) 
 except:
 continue #takes care of the game crashing with bad input
 if choice < 1 or choice > 9:
 continue #takes care of the game behaving oddly because of how python handles arrays
 space_check(board, choice)
 if space_check(board, choice) == False:
 good = True
 return choice
 return choice

Lastly, make sure you make comments on your code :)

• KeyesCode: Thank you for finding a flaw in my code. Thank you for showing me how to fix it and adding comments explaining what does what. I will start making a habit to adding comments in my code.
• reggaeguitar: Only add comments for "business logic" in the code, things that are unique to that program. Avoid obvious comments like "converts this string to uppercase"
• Jasmine: Yes, but if you have to choose, err on the side of more comments rather than fewer. Anyone who works on from your code in the future will thank you