HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinorCanonical

Tic Tac Toe game in Python

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
toetictacgamepython

Problem

Most of what I'm writing is pretty quick and dirty as I'm crashing through a lot of stuff to learn. My variable/function/class names are most likely not efficient and I still need to get a lot of the PEP8 formatting down.

That said, I put a quick Tic Tac Toe game together in Python. I'm going to add some decision logic for the computer to run blocks and common Tic Tac Toe strategy, but for now it's just a dumb chooser.

Can I consolidate a lot of the code here? I think a lot of it is repetitive and it seems to me it could be much more efficient.

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

'''

Basic Tic Tac Toe game - Chris Gleason 2015

'''

from random import randint

whowon = 'no'
coords = [ [ '[1]' , '[2]' , '[3]'] , [ '[4]' ,'[5]' , '[6]' ] , [ '[7]' , '[8]' , '[9]' ] ]

def printmatrix():

'''
Function to print out the Tic Tac Toe Matrix
'''

print (' This is the matrix of the co-ordinates ')
print ('''
''')

print ('' , coords[0][0] , '\t' , coords[0][1] , '\t' , coords[0][2])
print ('' , coords[1][0] , '\t' , coords[1][1] , '\t' , coords[1][2])
print ('' , coords[2][0] , '\t' , coords[2][1] , '\t' , coords[2][2])
print ('')

def wincondition():

'''
This Fucntion defines wether a player has won and if so who has won before allowing
execution of the playturn function.
'''

if coords[0][0] == "[X]" and coords [0][1] == "[X]" and coords[0][2] == "[X]":
iswon = 'yes'
whowon = 'player'

elif coords[1][0] == "[X]" and coords [1][1] == "[X]" and coords[1][2] == "[X]":
iswon = 'yes'
whowon = 'player'

elif coords[2][0] == "[X]" and coords [2][1] == "[X]" and coords[2][2] == "[X]":
iswon = 'yes'
whowon = 'player'

elif coords[0][0] == "[X]" and coords [1][0] == "[X]" and coords[2][0] == "[X]":
iswon = 'yes'
whowon = 'player'

elif coords[0][1] == "[X]" and coords [1][1] == "[X]" and coords[2][1] == "[X]":
iswon = 'yes'
whowon =

Solution


  • You are repeating yourself in playturn and in compturn.



A large chunk of these two functions is exactly the same thing: checking to see if a square is empty or not.

I recommend creating a function call is_available which will take one argument that is an element of coords, and will return True if the space is empty and False if not.

Here is what I came up with:

def is_available(coord):
    """
    @param(string) -- a coord from the coords array
    @return(boolean) -- True if the square is empty
                     -- False if the square is occupied
    """
    return coord != "[X]" && coord != "[O]"


I did something a little different in this function than what you did that allowed me to reduce this function to one line. You were checking if the square was equal to it's default value ("[1]", "[3]", etc) and I checked that the square was not X and was not O.

Now, let's go back to compturn. Let's re-write it using is_available:

def compturn():

'''
This function adds decision logic to the computers turn, picks a random number
and keeps looping until it picks a valid unchosen spot on the grid.
'''

wincondition()
goodroll = false
while not goodroll:
    choice1 = randint(0,2)
    choice2 = randint(0,2)
    if is_available(coords[choice1, choice2]):
        coords[choice1, choice2] = "[0]"
        goodroll = True

print('Computer choice is: ' , compchoice)


I also made a few more improvements to this function:

-
I changed goodroll to use True and False, rather than "yes" and "no" because boolean comparisons are much, much faster than string comparisons.

-
I moved the final print statement out of the while loop so, in the case of an invalid move, the move won't be printed.

-
I go rid of the else part because, if the conditional before it was not True, then goodroll would not have been set and will remain False.

You should use this same principle here for your wincondition function so you aren't being so repetitive there either.

Your game does not handle ties. It will just asking for a place to go (or randomly generating a place to go in the case of the computer's turn) and the program will never exit.

You should add a function that checks for a tie.

printmatrix is not a descriptive name. You should call it print_board.

Here:

if iswon == 'yes':
    print ('The game is over! ' ,  whowon , ' won!')
    exit(0)


You use the function exit, but you never imported it anywhere.

At the top of your code, you should put:

from sys import exit

Code Snippets

def is_available(coord):
    """
    @param(string) -- a coord from the coords array
    @return(boolean) -- True if the square is empty
                     -- False if the square is occupied
    """
    return coord != "[X]" && coord != "[O]"
'''
This function adds decision logic to the computers turn, picks a random number
and keeps looping until it picks a valid unchosen spot on the grid.
'''

wincondition()
goodroll = false
while not goodroll:
    choice1 = randint(0,2)
    choice2 = randint(0,2)
    if is_available(coords[choice1, choice2]):
        coords[choice1, choice2] = "[0]"
        goodroll = True

print('Computer choice is: ' , compchoice)
if iswon == 'yes':
    print ('The game is over! ' ,  whowon , ' won!')
    exit(0)
from sys import exit

Context

StackExchange Code Review Q#95801, answer score: 8

Revisions (0)

No revisions yet.