First Python program: Tic-Tac-Toe

Question

This is my first program written outside of using books and tutorials. Any help on style and functionality would be helpful.

import sys
from textwrap import dedent
import os
import random



os.system('CLS') 

# board number setup
board = [0,1,2,
         3,4,5,
         6,7,8]

# Defines the board layout printed to the console
def board_layout():
    print(dedent(f''' 
    *************
    * {board[0]} | {board[1]} | {board[2]} *
    *-----------*
    * {board[3]} | {board[4]} | {board[5]} *
    *-----------*
    * {board[6]} | {board[7]} | {board[8]} *
    *************
    '''))


move_count= 0
def main():
    while  True:

        #Prints board layout to console.
        board_layout()
        #checks for a winner when called at end of each turn
        def check_winner():
            global move_count
            #list of lists with all the winning combinations for from the tic tac toe board
            winning_list = [[board[0],board[1],board[2]],[board[3],board[4],board[5],],
            [board[6],board[7],board[8]],[board[0],board[4],board[8]],[board[2],board[4],board[6]],
            [board[0],board[3],board[6]],[board[1],board[4],board[7]],[board[2],board[5],board[8]]]
            #Keeps a reference to winning_list so it is updated at the end of every turn        
            new_list = winning_list

            #iterates over the lists in winning_list
            for i,j,k in winning_list:
                #looks at the lists in winning_list to determine if a list has all x's for a win
                if i ==  'x' and j ==  'x' and k ==  'x' :
                    print('X wins')
                    end()
                #looks at the lists in winning_list to determine if a list has all o's for a win
                elif i ==  'o' and j ==  'o' and k ==  'o' :
                    print('O wins')
                    end()
            #possible moves is 9 in tic tac toe. If all moves are taken and there is no winner no winner forces a draw. 
            if move_count == 9:
                print('You Tied')
                end()




        #Takes user input for the move
        move =int(input('Please select a spot: '))
        print(move)

        #Player move, makes sure the spot is not taken and adds 1 to move_count
        if board[move] !='x' and board[move] != 'o':
            board[move] = 'x'
            move_count += 1
            check_winner()  
            #npc move, chooses a random spot that is not taken and adds 1 to move_count
            while True:
                npc = random.randint(0,8)
                if board[npc] != 'o' and board[npc] != 'x':
                    board[npc] = 'o'
                    print('Computer chooses spot ', npc)
                    move_count += 1
                    check_winner()
                    break
        #If spot is taken prints that the spot is already taken
        else:
            print('This spot is taken')

        #Game ending
        def end():
            print('Thank you for playing')
            sys.exit()

if __name__ == "__main__":
    main()

Answer 1

The function check_winner() does not need global move_count. Using global is code smell, avoid if at all possible, which tends to be always. But in this case it is completely unnecessary, as move_count, like board, is already accessible in check_winner().

winning_list is constructed every time check_winner() is called. It does not persist from one call to the next, so new_list = winning_list and the comment immediately above it should be removed.

---

The statement move = int(input(...)) can crash if the user enters invalid input. Even if a valid integer is given, the integer could be outside the valid range, like 42, which will cause when board[move] is evaluated. Place user input in a try ... except statement, inside a loop, and don’t let the program continue until valid input has been given.

---

You have a game loop that handles two turns (a move by both players) each pass through the loop. While this does work, it will paint you into a corner in subsequent programs. 3 or more players is going to make writing the game loop much harder.

It is usually simpler to handle one turn (a move by only one player), in each pass through the loop. At the end of the loop, the “current player” is incremented, wrapping around to the first player when necessary. With only 2 players, this alternates between them. More advanced games may require skipping player when a “lose a turn” move is made. Other games may even reverse the direction of play mid game. All of these would be horrible to try to write the game loop for if each pass through the loop tried to handle all player moves in one pass.

---

When the game loop is change to handle only a single move at each pass, it is much easier to handle the “game over” condition. A while game_is_running loop is all that is required. Or, for tic-tac-toe, you could use:

for move_count in range(9):
    # moves made here
    # break if someone wins
else:
    print("You Tied")

The else: clause of a for loop only executes if the loop finishes without executing break, so after 9 moves with no winner, it is a tie game.

Using sys.exit() to stop the interpreter on a “game over” condition is a bad idea. It works here, but it makes test code impossible to write, because the program can kill the interpreter, and the test code can’t stop that.

Answer 2

Function Placement

You lose a bit of performance and readability by defining check_winner inside your while loop. move_count, board etc are all in global scope, even though they are within that loop:

def check_winner():
    # Rest of function

while True:

The def end() could also be moved to global scope, because again you are redefining it during every iteration which isn't what you want.

check_winner

The new_list = winning_list doesn't do anything, it copies the reference from winning_list and the two variables are tied together unless you did a deep_copy, which creates a new object. Furthermore, I don't really see any use of new_list anywhere, so you can just drop that line entirely.

As @AJNewfeld pointed out, the global move_count can be dropped because, again, move_count is already global and is accessible by all check_winner, as it will look in the locals() mapping first, if move_count isn't in the local mapping (from positional or keyword args taken in by the function), it will search globals(). A NameError is only raised when those don't contain the variable you are looking for.

Making Moves

The while loop for npc can be easily refactored so that you aren't possibly iterating over the entire board, and makes the code a bit easier to read. Your board is made up of either two entries: int for open spots and str for taken spots. This means that npc's move can be a function like so:

def npc_move():
    # This will give you only the indices for spots that have yet to be taken
    remaining_spots = [i for i, value in enumerate(board) if isinstance(value, int)]
    return random.choice(remaining_spots)

Or you could also use a set() globally to represent remaining spots and pop indices out of it:

# Declare globally at the beginning 
moves_left = set(range(9))

# Your while loop can now be to check if the set is empty or not
while moves_left: # A populated set acts as True
    my_move = moves_left.pop(random.choice(moves_left))

    # Now moves_left has one fewer element

Taking this idea a little further, you could combine the user's move with the npc's move in one function:

# The npc default will allow you to set it to True if it's
# npc's turn, otherwise, no args need to be supplied
def make_move(npc=False):

    if npc is False:
        user_move = "" # dummy default to kick off while loop
        while user_move not in moves_left:
            try:
                user_move = int(input(f"Choose a move out of {moves_left}: "))
                return moves_left.pop(user_move)
            except ValueError, KeyError: # invalid int conversion or not in moves_left
                print("Invalid move")
                continue

    else:
        return moves_left.pop(random.choice(moves_left))  

You can then call it like:

moves_left = set(range(9)) # At beginning of game

npc_move = make_move(npc=True)
3
user_move = make_move()

Choose a move out of {0, 1, 2, 4, 5, ,6 ,7, 8}: a
Invalid move
Choose a move out of {0, 1, 2, 4, 5, ,6 ,7, 8}: 3
Invalid move
Choose a move out of {0, 1, 2, 4, 5, ,6 ,7, 8}: 4

user_move
4
```

Answer 3

I made some of the changes suggested by @AJNeufeld. I made the game loop a for i in range(9) and removed all global variables from the code. Put the move by the player in a try/except block to catch IndexError and the loop only handles one turn each go throug and looping back to the beginning when needed(I am not sure if i did this is the best way). End no longer uses the sys.exit() and changed to quit() and now offers am option to play again.

import sys
from textwrap import dedent
import os
import random

os.system('CLS')

# board number setup
board = [0, 1, 2,
         3, 4, 5,
         6, 7, 8]


# Defines the board layout printed to the console
def board_layout():
    print(dedent(f''' 
    *************
    * {board[0]} | {board[1]} | {board[2]} *
    *-----------*
    * {board[3]} | {board[4]} | {board[5]} *
    *-----------*
    * {board[6]} | {board[7]} | {board[8]} *
    *************
    '''))


def main():
    players = ('Player','NPC')
    turn = 'Player'
    change_turn = 0
    for moves in range(9):
        if turn == 'Player':   
            while True:
                try:
                    board_layout()
                    player_move = int(input('Please select a spot: '))
                    if board[player_move] != 'x' and board[player_move] != 'o':
                        board[player_move] = 'x'
                        check_winner()
                        break
                except IndexError:
                    print('please select valid spot')
        if turn == 'NPC':
             # npc move, chooses a random spot that is not taken 
            while True:
                npc = random.randint(0, 8)
                if board[npc] != 'o' and board[npc] != 'x':
                    board[npc] = 'o'
                    print('Computer chooses spot ', npc)
                    check_winner()
                    break  
        try:
            change_turn += 1
            turn = players[change_turn]
        except:
            change_turn = 0
            turn = players[change_turn]
    else:
       print('You Tied')
       end()       





def end():
        print('Thank you for playing')
        answer = input('Would you like to play again?: Y/N')

        if answer.lower() == 'n':
            quit()
        elif answer.lower() == 'y':
                    clear_board()
                    main()
        else:
            print('Please choose a valid option')
            end()

def clear_board():
    for i in range(9):
        board[i] = i

# checks for a winner when called at end of each turn        
def check_winner():

            # list of lists with all the winning combinations for from the tic tac toe board
            winning_list = [[board[0], board[1], board[2]], [board[3], board[4], board[5], ],
                            [board[6], board[7], board[8]], [board[0], board[4], board[8]],
                            [board[2], board[4], board[6]],
                            [board[0], board[3], board[6]], [board[1], board[4], board[7]],
                            [board[2], board[5], board[8]]]

            # iterates over the lists in winning_list
            for i, j, k in winning_list:
                # looks at the lists in winning_list to determine if a list has all x's for a win
                if i == 'x' and j == 'x' and k == 'x':
                    print('X wins')
                    end()
                # looks at the lists in winning_list to determine if a list has all o's for a win
                elif i == 'o' and j == 'o' and k == 'o':
                    print('O wins')
                    end()


if __name__ == "__main__":
    main()