I wrote a simple TicTacToe console game in Python3. I would like to make my program more pythonic than it is now. For example, instead of if a == 5 and b == 5 and c == 5:
you can write if a == b == c == 5:
. So I expect hints of how can I rewrite my code to take adventage of Python language.
I don't ask for algorithm optimization (it contains some bugs, I know). The code should be rather understood for you except computer_turn()
method which simulates computer's move. But since I don't ask for optimization I don't see a need to explain what exacly happens inside this method.
You may find my code on Github.
from random import randint, choice
import time, sys, os
class TicTacToe:
board = [None for i in range(9)] # list representing game's board
turn = 1 # counter of computer's turns
def print_board(self, board = None): # printing game board
board = self.board if board == None else board
os.system("clear") # use "cls" for windows
print("|---|---|---|")
for i, cell in enumerate(board, 1):
print("| {} ".format(" " if cell == None else cell), end = "")
if i % 3 == 0:
print("|")
print("|---|---|---|")
def get_game_winner(self, board = None):
board = self.board if board == None else board
for i in range(0, 9, 3):
if board[i] == board[i + 1] == board[i + 2]:
if board[i] != None: return board[i]
for i in range(0, 3):
if board[i] == board[i + 3] == board[i + 6]:
if board[i] != None: return board[i]
if board[0] == board[4] == board[8] or board[2] == board[4] == board[6]:
if board[4] != None: return board[4]
return None
def computer_turn(self):
position = None
def get_line(cell):
if cell <= 3: return 1
if cell <= 6: return 2
return 3
if self.turn == 1:
self.turn = 2
oponent_choice = self.board.index("X")
if oponent_choice + 1 in [1, 3, 7, 9]:
position = 5 - 1
else:
position = choice([1, 3, 7, 9]) - 1
elif self.turn == 2:
self.turn = 3
for i in [1, 3, 7, 9]:
if self.board[i - 1] == "X":
if self.board[5 - 1] == "X":
position = 10 - i - 1
if self.board[position] == "O":
position = choice([item - 1 for item in [1, 3, 7, 9] if item not in [position + 1, i]])
break
for j in [1, 3, 7, 9]:
if i != j and self.board[j - 1] == "X":
position = i + (j - i) // 2 - 1
if self.board[position] == "O":
position = choice([2, 4, 6, 8]) - 1
break
for j in [2, 4, 6, 8]:
if self.board[j - 1] == "X":
diff = abs(j - i)
if get_line(i) != get_line(j) and diff == 1:
position = 5 - 1
else:
position = max(i, j) + diff
if position > 9: position = min(i, j) - diff
if self.board[position - 1] == "O": position = 5
position -= 1
if position == None:
for i in [2, 4, 6, 8]:
if self.board[i - 1] == "X":
if self.board[5 - 1] == "X":
position = 10 - i - 1
else:
position = 5 - 1
if self.turn > 2:
tmp_board = self.board[:]
candidates = [i for i in range(9) if self.board[i] == None]
for candidate in candidates:
tmp_board[candidate] = "O"
if self.get_game_winner(tmp_board) == "O":
position = candidate
break
tmp_board[candidate] = None
if position == None:
for candidate in candidates:
tmp_board[candidate] = "X"
if self.get_game_winner(tmp_board) == "X":
position = candidate
break
tmp_board[candidate] = None
if position == None: position = candidates[0]
self.board[position] = "O"
def ask_for_choice(self):
while True:
try:
print("Your choice: ", end = "")
position = int(input())
except ValueError:
print("That's not an integer!")
continue
if 1 <= position <= 9:
return position - 1
else:
print("Number must be in range 1 to 9!")
def user_turn(self):
position = self.ask_for_choice()
while self.board[position] != None:
print("This position is filled")
position = self.ask_for_choice()
self.board[position] = "X"
def is_game_finished(self):
if None not in self.board:
print("The game ended with a draw")
sys.exit()
winner = self.get_game_winner()
if winner == None: return
if winner == "X": print("You won the game!")
else: print("Computer won the game")
sys.exit()
def start(self):
self.print_board([i for i in range(1, 10)])
print()
print("Hello, the game just started!")
while True:
self.user_turn()
self.print_board()
self.is_game_finished()
time.sleep(0.5)
self.computer_turn()
self.print_board()
self.is_game_finished()
game = TicTacToe()
game.start()
2 Answers 2
It is better to write:
import time import sys import os
instead of import time, sys, os
(this is recommended PEP 8, in imports section)
- You declared
board
andturn
as class attributes, but they evolve through your program as instance variables instead. This is wrong per design. - Your program presents condition checking in the forms of
if something == None:
andif something != None:
13 times. But you can simply write:if not something:
andif something:
(respectively) instead. - In
ask_for_choice()
, you can safely get rid of thecontinue
instruction as this is already done by default given the way the exception is handled. - In
user_turn()
, it is more appropriate, in your context, to write:while self.board[position]:
instead ofwhile self.board[position] != None:
- In Python, if you do not specify a return value to a function, this later one will return
None
. This means you can safely get rid ofreturn None
instruction in theget_game_winner()
function. - In
is_game_finished()
, you can safely get rid ofif winner == None: return
because this achieved anyway if the other conditions are not met. - The reader of your code would expect the function
is_game_finished()
to return True or False, but this is not the case. You should either rename this function or modify its design. - In
start()
, you could writeprint("\nHello, the game just started!")
instead ofprint()
followed byprint("Hello, the game just started!")
. In the same time, there is nothing that justifies the blank line which exists in thewhile
statement. - Inner functions, in Python, are more appropriate to use to design decorators. I personally do not like the definition of a function called
get_line()
withincomputer_turn()
. - The
computer_turn()
function is, IMHO, quite long (but others may argue about it). I would have divided those tasks between smaller functions for the sake of easier unit testing and, more importantly, SRP. - Throughout your code, several empty lines are left where this is not justified. You may help yourself by reading blank lines on PEP8.
It would be nice to put "a guard" to your program. I mean something like this:
if __name__ == '__main__': game = TicTacToe() game.start()
You may read on StackOverflow: What does if name == "main": do?
-
\$\begingroup\$ Thank you for your response. I will try to fix all my mistakes. However, if I remove the
continue
statement fromask_for_choice()
method then the program will crash if you type non-integer value. To avoid that, I could declare variableposition
out of thetry block
. \$\endgroup\$mrJoe– mrJoe2017年11月26日 15:19:24 +00:00Commented Nov 26, 2017 at 15:19 -
2\$\begingroup\$
!something
? Do you meannot something
? \$\endgroup\$Richard Neumann– Richard Neumann2017年11月27日 10:18:53 +00:00Commented Nov 27, 2017 at 10:18
There are two primary use cases of range
: you want to feed in a variable as a parameter, or you have a large number of elements. When you have range(0, 9, 3)
, you're feeding in three fixed parameters to get an iterator with only three elements. It would be much more readable if you just put [0,3,6]
.
There are quite a few lines where I don't understand why you have the code you do, such as
oponent_choice + 1 in [1, 3, 7, 9]
instead of oponent_choice in [0, 2, 6, 8]
and
position = 5 - 1
instead of position = 4
(And while misspellings don't affect your code as long as you're consistent, "opponent" has two p's)
If you have the same list over and over again, you might want to name it. E.g. corners = [1,3,7,9]
On a broader level, an option would be to name each line in which a player can win, e.g. lines = ['top','middle','bottom','left','center','right','down','up']
, then create several dictionaries: squares_in_lines
has lines as keys and lists of squares in those lines as values, lines_in_squares
has squares as keys and lines that include those squares as values, and line_totals
has lines as keys and the sum of the squares in those lines as values, where 'X' is worth -1, 'O' is worth 1, and blank squares are 0. Every time someone makes a move, find what lines that square is in, and add the appropriate number to the appropriate line totals. If the absolute value of a line total reaches 3, the game is over. Assuming the computer is 'O', you can then have the computer turn check whether any line total is 2, and if so make a move in that line, otherwise check whether any line total is -2 and if so move in that line.