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()
\$\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?

\$\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\$
    – mrJoe
    Nov 26, 2017 at 15:19
  • 2
    \$\begingroup\$ !something? Do you mean not something? \$\endgroup\$ 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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.