4
\$\begingroup\$

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()
Simon Forsberg
59.7k9 gold badges157 silver badges311 bronze badges
asked Nov 26, 2017 at 13:50
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$
  • 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 and turn 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: and if something != None: 13 times. But you can simply write: if not something: and if something: (respectively) instead.
  • In ask_for_choice(), you can safely get rid of the continue 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 of while 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 of return None instruction in the get_game_winner() function.
  • In is_game_finished(), you can safely get rid of if 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 write print("\nHello, the game just started!")instead of print() followed by print("Hello, the game just started!"). In the same time, there is nothing that justifies the blank line which exists in the while 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() within computer_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?

answered Nov 26, 2017 at 15:01
\$\endgroup\$
2
  • \$\begingroup\$ Thank you for your response. I will try to fix all my mistakes. However, if I remove the continue statement from ask_for_choice() method then the program will crash if you type non-integer value. To avoid that, I could declare variable position out of the try block. \$\endgroup\$ Commented Nov 26, 2017 at 15:19
  • 2
    \$\begingroup\$ !something? Do you mean not something? \$\endgroup\$ Commented Nov 27, 2017 at 10:18
2
\$\begingroup\$

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.

answered Nov 27, 2017 at 17:25
\$\endgroup\$

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.