patternpythonModerate
Unbeatable Tic-Tac-Toe program seems difficult to read
Viewed 0 times
unbeatabletoereadticprogramseemstacdifficult
Problem
I've made an unbeatable Tic-Tac-Toe in Python 3.3. While it truly was unbeatable, it was an eyesore to look at and nigh impossible to read. That code is here.
I have since then optimized it for Python 2.7.5 as follows:
```
from random import randrange, random
from time import sleep
#First, setup the board!!
def draw_board():
print '', board[1], '|', board[2], '|', board[3], \
'\n-----------\n', \
'', board[4], '|', board[5], '|', board[6], \
'\n-----------\n', \
'', board[7], '|', board[8], '|', board[9], \
'\n'
#Next, Choose what team you're on!
def player_team():
team = raw_input('Do you want to be X or O? \n').upper()
print
if team == 'X':
return ['X', 'O']
elif team == 'O':
return ['O', 'X']
else:
print ('That is not a valid choice. Please try again \n')
return player_team()
#Next, Find out what player goes first!
def first_turn():
turn = random()
if turn <= .494:
print 'You will go first \n'
return ('user', True)
else:
print 'The Computer will go first \n'
return ('computer', False)
#Next, be able to determine which spaces are available!
def available(space):
if board[space] == ' ':
return True
#Next, allow a player to pick their move!
def user_turn():
try:
move = int(raw_input('Where would you like to move? (Enter a number from 1-9) \n'))
if 0 < move < 10:
if not available(move):
print ('That space is already taken by a player. '
'Please select an open space \n')
return user_turn()
else:
board[move] = user_team
print
except:
print 'That is not a valid move. Please try again. \n'
return user_turn()
#Now define the A.I!
def computer_turn():
move = randrange(1, 10)
if available(move):
board[move] = computer_team
return
I have since then optimized it for Python 2.7.5 as follows:
```
from random import randrange, random
from time import sleep
#First, setup the board!!
def draw_board():
print '', board[1], '|', board[2], '|', board[3], \
'\n-----------\n', \
'', board[4], '|', board[5], '|', board[6], \
'\n-----------\n', \
'', board[7], '|', board[8], '|', board[9], \
'\n'
#Next, Choose what team you're on!
def player_team():
team = raw_input('Do you want to be X or O? \n').upper()
if team == 'X':
return ['X', 'O']
elif team == 'O':
return ['O', 'X']
else:
print ('That is not a valid choice. Please try again \n')
return player_team()
#Next, Find out what player goes first!
def first_turn():
turn = random()
if turn <= .494:
print 'You will go first \n'
return ('user', True)
else:
print 'The Computer will go first \n'
return ('computer', False)
#Next, be able to determine which spaces are available!
def available(space):
if board[space] == ' ':
return True
#Next, allow a player to pick their move!
def user_turn():
try:
move = int(raw_input('Where would you like to move? (Enter a number from 1-9) \n'))
if 0 < move < 10:
if not available(move):
print ('That space is already taken by a player. '
'Please select an open space \n')
return user_turn()
else:
board[move] = user_team
except:
print 'That is not a valid move. Please try again. \n'
return user_turn()
#Now define the A.I!
def computer_turn():
move = randrange(1, 10)
if available(move):
board[move] = computer_team
return
Solution
-
Some of the comments are wrong, for example:
appears just before the
The best practice here is to write a docstring for each function explaining what it does, how to call it, and what kind of values it returns.
-
You use global variables
but this is not the global
Global variables also make it hard to test from the interactive interpreter, because you have to fiddle about getting the globals into the right state for whatever it is you want to test.
It is generally better if you pass your state as parameters to functions (for example, the
-
Your
Now it's straightforward to test it from the interactive interpreter:
-
Your function
(In real life,
Instead, you should use iteration, for example like this:
There are similar problems in
-
The
-
In the
what happens if
and
so
-
Your function
It would be better to redesign the interface so that it's easier to understand, for example like this:
-
The function
```
# The lega
Some of the comments are wrong, for example:
#First, setup the board!!appears just before the
draw_board function, which does not set up the board.The best practice here is to write a docstring for each function explaining what it does, how to call it, and what kind of values it returns.
-
You use global variables
board, user_win, computer_win, ties, user_team, computer_team, turn. Global variables make code hard to read, because you have to remember which variables are globals, and this might not be at all obvious. For example, the function first_turn starts with the line:turn = random()but this is not the global
turn, it's a local variable with the same name!Global variables also make it hard to test from the interactive interpreter, because you have to fiddle about getting the globals into the right state for whatever it is you want to test.
It is generally better if you pass your state as parameters to functions (for example, the
draw_board function could take board as a parameter), or maintain it as properties of objects (for example, user_win could be a property of an object).-
Your
draw_board function could benefit from using Python's string formatting interface. I would also rename it print_board (because I prefer to reserve the word draw for rendering graphics).def print_board(board):
"""Print 'board' to standard output."""
print(" {1} | {2} | {3}\n"
"---+---+---\n"
" {4} | {5} | {6}\n"
"---+---+---\n"
" {7} | {8} | {9}\n".format(*board))Now it's straightforward to test it from the interactive interpreter:
>>> print_board('.XOXXOOOXX')
X | O | X
---+---+---
X | O | O
---+---+---
O | X | X
-
Your function
player_team uses tail recursion to implement a potentially infinite loop. But Python does not support tail call elimination, so a player who insists on repeatedly entering a wrong choice will eventually overflow the Python stack and cause it to fail:>>> import sys
>>> sys.setrecursionlimit(3)
>>> player_team()
Do you want to be X or O?
No!
That is not a valid choice. Please try again
Do you want to be X or O?
No, thank you.
Traceback (most recent call last):
File "", line 1, in
File "cr33359.py", line 23, in player_team
return player_team()
File "cr33359.py", line 17, in player_team
if team == 'X':
RuntimeError: maximum recursion depth exceeded in cmp
(In real life,
sys.setrecursionlimit is larger than 3, so it would take somewhat longer to reach the maximum recursion depth, but a determined player could get there in the end.)Instead, you should use iteration, for example like this:
def player_teams():
"""Prompt user for their preferred symbol (X or O), repeatedly if
necessary, and return a string consisting of the player's symbol
and the computer's symbol.
"""
while True:
team = raw_input('Do you want to be X or O? ').upper()
if team == 'X': return 'XO'
elif team == 'O': return 'OX'
else: print("Please enter X or O.")There are similar problems in
user_turn, computer_turn, and play_again.-
The
first_turn function returns a pair whose second value (called strategy in the main loop) is unused. So why bother returning it?-
In the
available function:def available(space):
if board[space] == ' ':
return Truewhat happens if
board[space] is not a space? There's no else: clause, so the function just ends without explicitly returning anything. Luckily for you, "Falling off the end of a function returns None">>> def noreturn(): pass
...
>>> print(noreturn())
Noneand
None becomes False when converted to a Boolean:>>> bool(None)
Falseso
available works. But this seems needlessly tricky. It's simpler and clearer to write:def available(board, position):
"""Return True if 'position' is available to play on 'board'."""
return board[position] == ' '-
Your function
end_game returns the Boolean True if the game is over and there's a winner, the string 'Tie' if the game is over and there's no winner, and it falls off the end (thus returning None) if the game is not over. This is a confusing interface. If you had tried to write a docstring for the function, as recommended in point 1 above, that explained what it was supposed to return then the confusion would have become apparent.It would be better to redesign the interface so that it's easier to understand, for example like this:
def game_status(board):
"""Return the status of 'board' as one of the following strings:
'X' or 'O' if that player has three in a row;
'tie' if the board is full but neither player has won; or
'unfinished' otherwise.
"""-
The function
end_game seems rather complex. In tic-tac-toe there are just eight winning lines, so why not just list them? I would write:```
# The lega
Code Snippets
#First, setup the board!!turn = random()def print_board(board):
"""Print 'board' to standard output."""
print(" {1} | {2} | {3}\n"
"---+---+---\n"
" {4} | {5} | {6}\n"
"---+---+---\n"
" {7} | {8} | {9}\n".format(*board))def player_teams():
"""Prompt user for their preferred symbol (X or O), repeatedly if
necessary, and return a string consisting of the player's symbol
and the computer's symbol.
"""
while True:
team = raw_input('Do you want to be X or O? ').upper()
if team == 'X': return 'XO'
elif team == 'O': return 'OX'
else: print("Please enter X or O.")def available(space):
if board[space] == ' ':
return TrueContext
StackExchange Code Review Q#33359, answer score: 16
Revisions (0)
No revisions yet.