Ggthjy

I'm new to Python and I just finished a school assignment where I had to make a tictactoe game with focus on OOP. I would love to get some pointers on how I can clean up my code (especially my Tictactoe class) as it's pretty messy right now.

What I've done so far is comment as much as possible to make it understandable, but I feel like my tictactoe class is a mess and I would like to get some pointers on how I can optimize it.

The class below enforces the rules and draws the board:

import os

class Board:



    board = [0,1,2,3,4,5,6,7,8,9]

    win_combinations = [

        (1,2,3),

        (4,5,6),

        (7,8,9),

        (1,5,9),

        (3,5,7),

        (1,4,7),

        (2,5,8),

        (3,6,9),

    ]

    GameOver = False



    #Draws the board

    def drawboard(self): 

        print('=========')

        print(self.board[7], '|', self.board[8], '|', self.board[9])

        print(self.board[4], '|', self.board[5], '|', self.board[6])

        print(self.board[1], '|', self.board[2], '|', self.board[3])

        print('=========')



    #Checks if the move the player just made, made him/she win the game

    def checkIfWon(self, choice):



        for a, b, c in self.win_combinations:

            if self.board[a] == self.board[b] == self.board[c]:

                print('Game over, player ' + choice + ' won the game')

                self.GameOver = True



    #Update the current board 

    def update(self, input, choice):



        self.board[input] = choice

        os.system('clear')

        self.drawboard()

        self.checkIfWon(choice)



    #Resets the board

    def resetBoard(self):

        self.board = [0,1,2,3,4,5,6,7,8,9]



    #Stops the game if tie

    def tie(self):

        list = []

        for x in self.board:

            if type(x) != int:

                list.append(x)



        if len(list) == 9:

            return True

The class below contains the runGame method which starts the game:

import os

from board import Board



class Tictactoe():



    b = Board()

    choicePlayer1 = ''

    choucePlayer2 = ''

    corretChoice = False

    correctPlayer1 = False

    correctPlayer2 = False





    def runGame(self):

        os.system('clear')



        #Resets the game when a new game is started

        #Is necessary if the players wish to play more than 1 game

        resetGame(self)



        #Makes sure the game only starts if player1 picks X or O

        while self.corretChoice == False:



            self.choicePlayer1 = input('Do you want to play X or O? ')

            print()

            if self.choicePlayer1 == 'X':

                self.choicePlayer2 = 'O'

                self.corretChoice = True

                print('Starting player selected X')



            elif self.choicePlayer1 == 'O':

                self.choicePlayer2 = 'X'

                self.corretChoice = True

                print('Starting player selected O')

            else:

                print('ERROR - input has to be either X or O!')

                continue



        os.system('clear')

        self.b.drawboard()



        while self.b.GameOver == False:

            self.correctPlayer1 = False

            self.correctPlayer2 = False



            #For player1

            while self.correctPlayer1 == False:

                while True:

                    try:

                        x = int(input(self.choicePlayer1 + ' Where do you want to place your piece? '))

                        break

                    except:

                        print('Input has to be a number, try again')



                if x > 0 and x < 10 and type(self.b.board[x]) != str:

                    self.b.update(x, self.choicePlayer1)

                    self.correctPlayer1 = True

                elif x == 10:

                    quit()

                else: 

                    print('Spot is taken, try again: ')



                if self.b.GameOver == True:

                    self.correctPlayer2 = True



                if self.b.tie() == True:

                    self.correctPlayer2 = True

                    self.b.GameOver = True

                    print('Game is a tie')

            #For player2

            while self.correctPlayer2 == False:

                while True:

                    try:

                        x = int(input(self.choicePlayer2 + ' Where do you want to place your piece? '))

                        break

                    except:

                        print('Input has to be a number, try again')



                if x > 0 and x < 10 and type(self.b.board[x]) != str:

                    self.b.update(x, self.choicePlayer2)

                    self.correctPlayer2 = True

                elif x == 10:

                    quit()

                else: 

                    print('Spot is taken, try again: ')



                if self.b.tie() == True:

                    self.b.gameOver = True

                    print('Game is a tie')



#Resets the game if the players wishes to play again

def resetGame(self):

    self.b = Board()

    self.choicePlayer1 = ''

    self.choucePlayer2 = ''

    self.corretChoice = False

    self.correctPlayer1 = False

    self.correctPlayer2 = False

    self.b.resetBoard()

The script i run to start the game:

from tictac import Tictactoe



run = Tictactoe()



while True:

    run.runGame()

    if input("Play again? (y/n)") == "n":

        quit()

Now, as has been said before: this is more of a codereview than a question. Nevertheless:

1. You're defining class Board: without parantheses and class Tictactoe() with. No big deal reall, but a bit of inconsistency. I personally just put parens on every declaration. Rids you of having to think about it.


2. The variables board, win_combinations and GameOver are defined as class variables since they're declared in the body of the class definition itself. This means that should you for some reason instantiate two games at once they'll mess with each other. It would be better to put them into a constructor __init__(self) (the actual constructor is __new__ but you hardly need to edit that so init is generally refered to as constructor). That way every instance has their own instances of these variables.


3. The names of the variables and general format doesn't conform to PEP8. Some things here are: Docstrings for classes to describe what they do etc.; variables named snake_case (except for globals/constants) and classes CamelCase; two lines between top level classes, functions etc.; double quotes rather than single quotes. Also the variables probably aren't supposed to be manipulated from outside the class so you could tell other programmers so by prefixing them with an underscore "_".


4. Maybe add some additional abstraction. The board in your case isn't only a board but also includes gamelogic like checking for a win etc.


5. You're using + for string concatenation/interpolation which is deprecated print('Game over, player ' + choice + ' won the game'). The more modern and also more efficient way would be to use an f-string f"Game over, player {choice} won the game".


6. 
   

   The block

   
   
   

   def tie(self):
   
       list = []
   
       for x in self.board:
   
           if type(x) != int:
   
               list.append(x)
   
   

   
   
   

   could be written as a list comprehension or generator expression: list = [x for x in self.board if type(x) != int]. Or using the filtermethod: list = list(filter(lambda x: type(x) != int, self.board)). You should also rename list to list_ or an actual expressive name saying what it represents here. And in the same function you could just return len(list) == 9 since that already is a boolean expression.

   
   

That's it for the board class. Most of the stuff like format, not using class variables etc also applies to the TicTacToe class.

1. 
   resetGame(self) should probably be a method on the class so you can do self.reset_game() or similar.


2. Typo in corretChoice -> correct_choice.


3. 
   b is a really inexpressive variable name for the board. Why don't you name it board? Especially if it's used across the whole class (rather than being just a local variable) that would make the code a lot clearer.


4. 
   

   You're catching everything here:

   
   
   

   try:
   
       x = int(input(self.choicePlayer1 + ' Where do you want to place your piece? '))
   
       break
   
   except:
   
       print('Input has to be a number, try again')
   
   

   
   
   

   which is really bad style. It will for example also catch stuff like keyboard interrupts. Since you want to catch errors in the conversion what you probably want is except ValueError:.

   
   

And lastly not really an error but: if the user inputs anything other than n on the "play again?"-prompt it'll restart.

I also feel like the runGame method is way too large - I'll see if I can come up with a clearer solution and post it up here if I can.

EDIT:
I've tried my hand at refactoring you're code:

import os



class Board():

    """Represents the game-board"""

    def __init__(self):

        self.board = [i for i in range(10)]

        self._win_combinations = [

            (1, 2, 3),

            (4, 5, 6),

            (7, 8, 9),

            (1, 5, 9),

            (3, 5, 7),

            (1, 4, 7),

            (2, 5, 8),

            (3, 6, 9)]

        self.game_over = False



    def draw_board(self):

        """Draws the board to the terminal"""

        print("=========")

        print(self.board[7], "|", self.board[8], "|", self.board[9])

        print(self.board[4], "|", self.board[5], "|", self.board[6])

        print(self.board[1], "|", self.board[2], "|", self.board[3])

        print("=========")



    def check_if_won(self, player):

        """Checks if the move the player just made, made him/her win the game"""

        for a, b, c in self._win_combinations:

            if self.board[a] == self.board[b] == self.board[c]:

                print(f"Game over, player {player} won the game")

                self.game_over = True



    def update(self, input, choice):

        """Update the current board"""

        self.board[input] = choice

        os.system("clear")

        self.draw_board()

        self.check_if_won(choice)



    def reset_board(self):

        """Resets the board"""

        self.board = [i for i in range(10)]



    def tie(self):

        """Stops the game if tie"""

        list_ = list(filter(lambda x: type(x) != int, self.board))

        return len(list_) == 9





class TicTacToe():

    def __init__(self):

        os.system("clear")

        self.board = Board()

        self.player_1_char = ""

        self.player_2_char = ""

        self.corret_choice = False

        self.get_player_char()



    def reset(self):

        """Resets the internal state to prepare for a new game"""

        self.player_1_char = ""

        self.player_2_char = ""

        self.board.reset_board()



    def get_player_char(self):

        """Ask the player what character he wants to use and verify choice"""

        while True:

            player_1_char = input("Do you want to play X or O? ")

            print()

            if player_1_char == "X":

                self.player_1_char = "X"

                self.player_2_char = "O"

                print("Starting player selected X")

                break

            elif player_1_char == "O":

                self.player_1_char = "O"

                self.player_2_char = "X"

                print("Starting player selected O")

                break

            else:

                print("ERROR - input has to be either X or O!")

        os.system("clear")



    def get_player_input(self, player_char):

        while True:

            while True:

                x = input(f"{player_char} Where do you want to place your piece?")

                if x.isdigit():

                    x = int(x)

                    break

                else:

                    print("Input has to be a number, try again")



            if x > 0 and x < 10 and type(self.board.board[x]) != str:

                self.board.update(x, player_char)

                break

            elif x == 10:

                quit()

            else: 

                print("Spot is taken, try again: ")



    def check_tie(self):

        if self.board.tie():

            self.board.game_over = True

            print("Game is a tie")

            return True

        return False



    def run(self):

        self.board.draw_board()



        while not self.board.game_over:

            self.correct_player_1 = False

            self.correct_player_2 = False



            self.get_player_input(self.player_1_char)

            if self.board.game_over:

                break

            if self.check_tie():

                break



            self.get_player_input(self.player_2_char)

            if self.board.game_over:

                break

            if self.check_tie():

                break





while True:

    TicTacToe().run()



    user_input = "a"

    while user_input not in "ny":

        user_input = input("Play again? (y/n)").lower()



    if user_input == "y":

        continue

    else:

        break