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. =)

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