# First Python program: Tic-Tac-Toe

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

• Welcome to Code Review! I hope you get some great answers. – Phrancis Apr 11 at 0:42
• Thank you Phrancis, I'm looking forward to learning from people in the field. – andrewtw Apr 11 at 1:17
• One possible algorithmic change in check_winner() would be checking only those lines where the move has been just made. Not sure, though, if it is an improvement - it would certainly save some effort on checking lines which did not change recently, but it would require additional work on selecting those needing checking. However, in games on bigger boards it will definitely make some gain. – CiaPan Apr 11 at 16:13
• Thank you for the clarification @Sᴀᴍ Onᴇᴌᴀ. I added it as an answer and explained the changes and mentioned who gave me the suggestions I used. – andrewtw Apr 11 at 22:05

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

• @AJNeufed Thank you for the input and i will work on the suggestion made. I will work on the suggestions and post an updated version – andrewtw Apr 11 at 7:29

## 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
$$$$


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

quit()
clear_board()
main()
else:
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()
`
• Are you happy with this new code, or are you hoping for additional review feedback? If the latter, you need to ask a new question, say “First Python program: Tic-Tac-Toe (Followup)”, include a link back to this question, and a link from this question to the follow up question. See “What should I do when someone answers my question” in “Help” for more details. All the feedback I can give you in a comment on this answer is “I don’t like what you’ve done”, but don’t have nearly the space to explain why. – AJNeufeld Apr 12 at 1:55
• @ AJNeufeld 52 i was hoping for addition feedback. I have posted a follow-up at link – andrewtw Apr 12 at 2:48