4
\$\begingroup\$

I’d love some feedback on this human v. human, tic tac toe game I wrote in Python 2.7.10. I’d love to hear your thoughts on optimization and making the code simpler and more "pythonic".

Also, I’m wondering if passing the player and turn variables around the way I have is the best approach, or if I should consider global variables (which is how I might do it in Javascript, in a closure).

# Command-line Tic Tac Toe for two humans written in Python.
# Play in a terminal by running 'python tictactoe.py'.
boxes = [ ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ]
first_player = 'X'
turn = 1
winning_combos = [ [0, 1, 2], [3, 4, 5], [6, 7, 8], [0, 3, 6],
 [1, 4, 7], [2, 5, 8], [0, 4, 8], [2, 4, 6], ]
def print_board(initial=False):
 """ Print the game board. If this is the beginning of the game,
 print out 1-9 in the boxes to show players how to pick a
 box. Otherwise, update each box with X or 0 from boxes[].
 """
 print('''
 {} | {} | {} 
 -----------
 {} | {} | {}
 -----------
 {} | {} | {} 
 ''').format(*([x for x in range(1, 10)] if initial else boxes))
def take_turn(player, turn):
 """ Create a loop that keeps asking the current player for
 their input until a valid choice is made.
 """
 while True:
 box = raw_input('Player %s, type a number from 1-9 to select a box: ' % player)
 try:
 box = int(box) - 1 # subtract 1 to sync with boxes[] index numbers
 if 0 <= box <= 8:
 if boxes[box] == ' ': # initial value
 boxes[box] = player # set to value of current player
 break
 else:
 print('That box is already marked, try again.\n')
 continue
 else:
 print('That number is out of range, try again.\n')
 continue
 except ValueError:
 # Not an integer
 print('That\'s not a valid number, try again.\n')
 continue
def switch_player(turn):
 """ Switch the player based on how many moves have been made.
 X starts the game so if this turn # is even, it's 0's turn. 
 """
 current_player = '0' if turn % 2 == 0 else 'X'
 return current_player
def check_for_win(player, turn):
 """ Check for a win (or a tie). For each combo in winning_combos[],
 count how many of its corresponding squares have the current 
 player's mark. If a player's score count reaches 3, return a win.
 If it doesn't, and this is already turn # 9, return a tie. If
 neither, return False so the game continues.
 """
 win = False
 tie = False
 if turn > 4: # need at least 5 moves before a win is possible
 for x in range(len(winning_combos)):
 score = 0
 for y in range(len(winning_combos[x])):
 if boxes[winning_combos[x][y]] == player:
 score += 1
 if score == 3:
 win = True
 if turn == 9:
 tie = True
 return win, tie
def play(player, turn):
 """ Create a loop that keeps the game in play
 until it ends in a win or tie
 """
 while True:
 take_turn(player, turn)
 print_board()
 win, tie = check_for_win(player, turn)
 if win or tie:
 if win:
 print('Game over. %s wins!\n' % player)
 else:
 print('Game over. It\'s a tie.\n')
 break
 turn += 1
 player = switch_player(turn)
# Begin the game: 
print('\n\nWelcome to Tic Tac Toe for two humans!')
print_board(initial=True)
play(first_player, turn)
janos
113k15 gold badges154 silver badges396 bronze badges
asked Feb 26, 2016 at 16:10
\$\endgroup\$
0

2 Answers 2

1
\$\begingroup\$

Avoid too much code in try blocks

The try ... except ValueError in the take_turn function wraps too many statements that it shouldn't need. Only the int(...) statement may raise a ValueError there, so it would be better to wrap only that in the try. When you wrap a large block of code, it can lead to masked errors or make defect localization harder.

Magic values

Instead of referring to the player symbols by their literals like '0' and 'X', it would be better to give them names.

Iterate over elements when possible

In this code you iterate over indexes of elements:

 for x in range(len(winning_combos)):
 score = 0
 for y in range(len(winning_combos[x])):
 if boxes[winning_combos[x][y]] == player:
 score += 1
 if score == 3:
 win = True

It would be more natural, simpler and safer to iterate over elements:

for combo in winning_combos:
 score = 0
 for index in combo:
 if boxes[index] == player:
 # ...
answered Feb 26, 2016 at 16:35
\$\endgroup\$
1
  • \$\begingroup\$ Thanks, that's helpful. That makes sense about try ... except and yes, that iteration is much more obvious and simpler. Mine is lifted from a javascript mindset, I see now. \$\endgroup\$ Commented Feb 26, 2016 at 17:58
1
\$\begingroup\$

If I could make one suggestion, it would be to tab in everything one more level and make it a class. This gives you the ability to call the program from the command line, using if __name__ == "__main__" but also load it as a module in the interpreter and play around with it as a malleable component. With as little overhead as python has in making a class vs a collection of functions, I've found very little reason not to start out with constructing a class right off the bat.

answered Feb 26, 2016 at 20:10
\$\endgroup\$
0

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.