I'm learning Python at the moment. While taking the first steps, I implemented a Tic Tac Toe game and wanted to share it here, to get some feedback and continue learning. I'm looking for any improvements within my code: style, common mistakes, etc.
def create_playing_board(side_length):
return [[0 for x in xrange(side_length)] for x in xrange(side_length)]
def print_current_board(board):
print "\n", " Y | ", board[0][2], '|', board[1][2], '|', board[2][2]
print " | --+---+--"
print " | ", board[0][1], '|', board[1][1], '|', board[2][1]
print " | --+---+--"
print " | ", board[0][0], '|', board[1][0], '|', board[2][0]
print " | "
print " ------------ X"
def set_marker(player, board):
print "\nplayer",player, " - x/y input between 0 and 2"
x = y = 3
while(x not in range(0,3)):
x = input("x: ")
while(y not in range(0,3)):
y = input("y: ")
if board[x][y] == 0:
board[x][y] = player
return board
else:
print '\nfield already used - choose again!'
set_marker(player,board)
return board
def check_for_winner(player, board):
if (board[0][0] == board[1][0] == board[2][0] == player):
return 1
elif (board[0][1] == board[1][1] == board[2][1] == player):
return 1
elif (board[0][2] == board[1][2] == board[2][2] == player):
return 1
elif (board[0][0] == board[0][1] == board[0][2] == player):
return 1
elif (board[1][0] == board[1][1] == board[1][2] == player):
return 1
elif (board[2][0] == board[2][1] == board[2][2] == player):
return 1
elif (board[0][0] == board[1][1] == board[2][2] == player):
return 1
elif (board[0][2] == board[1][1] == board[2][0] == player):
return 1
else:
return 0
board = create_playing_board(3)
player = 1
is_win = 0
print "welcome to tic tac toe"
while(is_win == 0):
print_current_board(board)
board = set_marker(player, board)
is_win = check_for_winner(player, board)
if is_win == 1:
print ' \n### player', player,' wins! ###\n'
print_current_board(board)
print "\ndo you guys want to play again?"
is_win = input("0 = yes // 1 = no: ")
if is_win == 0:
board = create_playing_board(3)
player = (player%2)+1
1 Answer 1
Note:
This answer has gone through several updates... Some areas are updated to enhance clarity.
Although it looks really good, I have a few suggestions.
- Use
print()
instead ofprint
if you want this to work with Python 3.x range(3)
is the same asrange(0,3)
- Don't use
is_win
to determine if the player wants to play again. Use an appropriate variable name. Try considering something likeplay_again
.- I actually suggest using
while True:
and breaking out of the while loop if the user says they don't want to play again instead. But you can make it work either way.
- I actually suggest using
- When you want to test your code, consider using this code below:
if __name__ == '__main__':
# play with stuff I created
# ....
- For
create_playing_board(side_length)
- I suggest getting rid of
side_length
because in a simple tic-tac-toe game, it's always 3 x 3. - Consider using different variable for the second generator. Like so:
- I suggest getting rid of
return [[0 for x in range(side_length)] for y in range(side_length)] # Either this
return [[0 for x in range(3)] for y in range(3)] # or this.
input()
does not cast the user's input into anything except string. So if you want to compare it with an integer, you need to cast it to integer usingint()
. See the code line below for a quick way to capture and cast into an integer:
int(input("A question to ask"))
- When you ask the player if he or she wants to play again and plan on storing their answer into
is_win
, I don't think that's a really good idea becauseis_win
is already being used for something else. I would consider this instead:
print("\nDo you guys want to play again?") # Ask if the user would like to play again.
play_again = int(input("0 = yes // 1 = no: "))
Update #1:
I just caught a couple of things...
(削除) How do you determine which player is the winner? (削除ここまで)-- Never mind.... I found how you did it.- The game just keeps on going, even though the board is filled and there is no winner. Try to add something to check for "It's a tie!"
Update #2:
I noticed a few more opportunities for improvement...
I suggest using
boolean
values instead of integer to determine one of the two options like yes / no.A way to think about this is:
If I want to check for a condition, I just want a "Yes" or a "No".
-- To check for this in programming, it's a "True" or a "False"See the code below:
# is_win = 0 # Instead of this...
is_win = False # Use this.
# .... Somewhere in your code ....
# This is just a snippet...
def check_for_winner(player, board):
if board[0][0] == board[1][0] == board[2][0] == player:
# return 1 # Instead of this...
return True # Do this.
# .... Somewhere in your code ....
# if is_win == 1: # Instead of this...
if is_win: # Use this...
- I found way to check for a Tie. I've created a new Gist for this example. Feel free to see how you can implement this check_for_tie() method in your code:
def check_for_tie(board):
if not any(0 in sublist for sublist in board):
return True
return False
If you plan on implementing #2, consider modifying the code you used to check for
is_win
to also check for whether there is a tie, and to print the result while minimizing unnecessary redundancy.Think about simplifying the
check_for_winner()
method. Check this code block below:
def check_for_winner(player, board):
# Diagonal
# This is placed up here first because this runs
# slightly quicker than the one in the for loop.
if board[0][0] == board[1][1] == board[2][2] == player or \
board[2][0] == board[1][1] == board[0][2] == player:
return True
# Horizontal or Vertical
for x in range(3):
if board[x][0] == board[x][1] == board[x][2] == player or \
board[0][x] == board[1][x] == board[2][x] == player:
return True
return False
Update #3:
Thousands of apologies... I've made quite a list of changes, but I'm just trying to help you out and improve my answer based on Conor Mancone's suggestion.
I've updated my Gist and created a new one for comparison.
- This Gist -- the same one I posted earlier in this answer has my up-to-date suggestion.
- The 'That' Gist has my alternate suggestion (slight differences).
Note:
Even though I suggested a laundry list of things in this answer, they are not all implemented in my Gist links; however, they can be taken into reflection.
I know this is probably close to a novel, but I hope this helps :)
-
\$\begingroup\$ thanks a lot for this detailed breakdown - ill rework the code with your input and will post it as soon as im finished :) just wanted to let you know that i really appreciate it :) \$\endgroup\$hi im vinzent– hi im vinzent2017年07月13日 16:28:41 +00:00Commented Jul 13, 2017 at 16:28
-
1\$\begingroup\$ @hiimvinzent I updated the code for the
check_for_winner()
method. -- It's at the end of my answer. \$\endgroup\$Sometowngeek– Sometowngeek2017年07月13日 16:40:47 +00:00Commented Jul 13, 2017 at 16:40 -
1\$\begingroup\$ Great review. One problem: your suggested
check_for_winner
function doesn't check the diagonals. Also, I personally try to avoidwhile True
(and the like), as it implicitly relies upon the contents of the loop to keep you from getting stuck in an infinite loop. That disconnect can make it easy to accidentally create an infinite loop down the road. I like to keep it deterministic when I can. In this case, tic-tac-toe has a maximum of 9 moves, so you could do afor turn in range(9)
. This will also remove the need to check for a tie (it's a tie if you finish all 9 turns). \$\endgroup\$Conor Mancone– Conor Mancone2017年07月13日 17:21:45 +00:00Commented Jul 13, 2017 at 17:21 -
1\$\begingroup\$ Now you're thinking like a real developer :). I suggest using
str.isdigit()
See this Gist on how to implement it. \$\endgroup\$Sometowngeek– Sometowngeek2017年07月14日 15:55:05 +00:00Commented Jul 14, 2017 at 15:55 -
1\$\begingroup\$ @hiimvinzent I just thought of a pretty cool idea. This might be a good challenge for you. Turn the tic tac toe game into a class. I think it'll be a fun challenge :) \$\endgroup\$Sometowngeek– Sometowngeek2017年07月16日 17:37:45 +00:00Commented Jul 16, 2017 at 17:37