patternpythonMinor

Improving and optimizing Tic-Tac-Toe game written in Python 3

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

writtentoetictacgameoptimizingpythonandimproving

Problem

I've been teaching myself Python3 for about a week now and I decided to put my skills to the test by writing a simple Tic-Tac-Toe game. Here is what I came up with:

```
#!/usr/bin/env python3

import random

board = [0]*9

def resetBoard():
for index, i in enumerate(board):
board[index] = 0

def printBoard():
counter = 0
s = ""
print("|-----|-----|-----|")
for index, i in enumerate(board):
s += "| "
if(board[index] == 0):
s += str(index)
if(board[index] == 1):
s+= "x"
if(board[index] == 2):
s += "o"
s += " "
counter += 1
if(counter == 3):
s += "|"
print(s)
print("|-----|-----|-----|")
s = ""
counter = 0

def getPlayers():
return input("1 or 2 Players?")

def moveSquare(xOro, ai):
square = 0
flag = 0
while(board[square] != 0 or flag == 0):
if(ai == 0):
if(xOro == 1):
square = eval(input("Choose a square to place X: "))
if(xOro == 2):
square = eval(input("Choose a square to place O: "))
if(ai == 1):
square = random.randint(0, 8)
flag = 1
board[square] = xOro
print("\n\n")

def checkForWin():
if(board[0] == board[1] and board[1] == board[2] and board[2] != 0):
return 1
if(board[3] == board[4] and board[4] == board[5] and board[5] != 0):
return 1
if(board[6] == board[7] and board[7] == board[8] and board[8] != 0):
return 1
if(board[0] == board[3] and board[3] == board[6] and board[6] != 0):
return 1
if(board[1] == board[4] and board[4] == board[7] and board[7] != 0):
return 1
if(board[2] == board[5] and board[5] == board[8] and board[8] != 0):
return 1
if(board[0] == board[4] and board[4] == board[8] and board[8] != 0):
return 1
if(board[2] == board[4] and board[4] == board[6] and board[6]

Solution

#!/usr/bin/env python3

import random

board = [0]*9

Having global variables that change is considered a bad idea. You'd be better off putting this variable and the methods inside a class.

def resetBoard():
    for index, i in enumerate(board):
        board[index] = 0

You could also do board[:] = [0] * 9

def printBoard():
    counter = 0

In python you rarely need to keep track of counters like this

s = ""

In python, you should avoid building strings by adding different pieces together

print("|-----|-----|-----|")
    for index, i in enumerate(board):
        s += "|  "
        if(board[index] == 0):

The point of enumerate is that i is the value in board. Use it rather then refetching it here.

s += str(index)
        if(board[index] == 1):
            s+= "x"
        if(board[index] == 2):
            s += "o"
        s += "  "

By building the string this way its hard to follow exactly what the string is.

counter += 1
        if(counter == 3):

Instead of making the counter use

if index % 3 == 2:' Also you don't need the parens

s += "|"
            print(s)
            print("|-----|-----|-----|")
            s = ""
            counter = 0


Your whole algorithm here is rendered more complicated because you are iterating over cells rather the rows. Your algorithm would be much more natural with rows

def getPlayers():
    return input("1 or 2 Players?")


Shouldn't this return an int, not a string?

def moveSquare(xOro, ai):


variable names are recommended to follow x_or_o as a naming convetion

square = 0
    flag = 0


If this is a boolean flag, use False not 0

while(board[square] != 0 or flag == 0):


No need for the parens around the while expression

if(ai == 0):


Again use True/False for boolean flags. No need for parens

if(xOro == 1):
                square = eval(input("Choose a square to place X: "))
            if(xOro == 2):
                square = eval(input("Choose a square to place O: "))


Avoid magic numbers like 1 and 2 to denote X and O. In this case, I'd probably pass 'x' or 'o' strings.

if(ai == 1):


Use an else:

square = random.randint(0, 8)
        flag = 1


Flag is really terrible variable name because I don't know what it means. Boolean flags are usually an ugly solution

board[square] = xOro
    print("\n\n")

def checkForWin():
    if(board[0] == board[1] and board[1] == board[2] and board[2] != 0):
        return 1


Don't use magic numbers like that. Instead create some global constants to return or something

if(board[3] == board[4] and board[4] == board[5] and board[5] != 0):
        return 1
    if(board[6] == board[7] and board[7] == board[8] and board[8] != 0):
        return 1
    if(board[0] == board[3] and board[3] == board[6] and board[6] != 0):
        return 1
    if(board[1] == board[4] and board[4] == board[7] and board[7] != 0):
        return 1
    if(board[2] == board[5] and board[5] == board[8] and board[8] != 0):
        return 1
    if(board[0] == board[4] and board[4] == board[8] and board[8] != 0):
        return 1
    if(board[2] == board[4] and board[4] == board[6] and board[6] != 0):
        return 1


A lot duplication here. 

for index, i in enumerate(board):
        if(i  == 0):
            break
        if(index == 8):
            return 2
    return 0

def gameOver():
    return eval(input("\nPress 0 and then ENTER to play again!"))


Eval is a bit dangerous because the user can put in anything they want.

# Main


You should your main logic in a function

random.seed()
quit = 0
while(quit == 0):


Make quite a boolean variable

players = 0
    win = 0
    while(players != 1 and players != 2):
        players = eval(getPlayers())
    resetBoard()
    game = 0 #game = 1 when game is finished


Game becoming 1 when the game is finished seems backwards

while(game == 0): #game loop
        printBoard()
        moveSquare(1, 0)
        game = checkForWin()
        if(game > 0):
            win = game
            break


Not much point checking game == 0 above if you are just going to break anyways

printBoard()
        if(players == 1):
            moveSquare(2, 1)
        if(players == 2):
            moveSquare(2, 0)
        game = checkForWin()
        if(game > 0):
            win = game + 1
            break


See how the two parts of the loop are very similar? Suggests that you should get rid of the duplication.

if(win == 1):
        print("\nX Wins!")
    if(win == 2):
        print("\nO Wins!")
    if(win == 3):
        print("\n The game is a draw!")
    quit = gameOver()


Having gameOver() return whether or not you should quit seems confusing. The name doesn't suggest that

exit()


Not much point in calling quit, the program is about to terminate anyways

And just to demonstrate how I'd do this:

``
#!/usr/bin/env python3
import

Code Snippets

#!/usr/bin/env python3

import random

board = [0]*9

def resetBoard():
    for index, i in enumerate(board):
        board[index] = 0

def printBoard():
    counter = 0

print("|-----|-----|-----|")
    for index, i in enumerate(board):
        s += "|  "
        if(board[index] == 0):

s += str(index)
        if(board[index] == 1):
            s+= "x"
        if(board[index] == 2):
            s += "o"
        s += "  "

Context

StackExchange Code Review Q#5397, answer score: 5

Revisions (0)

No revisions yet.