Python tic tac toe class

Question

I'm fairly new at programming and made a tic tac toe game to see if I could handle it. I managed to get it to work but I feel like my code is faaaar from perfection. I wanted to ask for some advices on how to improve it

class Game:

    def __init__(self):
        self.board = [0, 1, 2, 3, 4, 5, 6, 7, 8]
        self.win_conditions = ((0, 1, 2), (3, 4, 5), (6, 7, 8), (0, 4, 8), (2, 4, 6), (0, 3, 6), (1, 4, 7), (2, 5, 8))
        self.moves_count = 0

    def create_board(self):
        print()
        print(self.board[0], self.board[1], self.board[2])
        print(self.board[3], self.board[4], self.board[5])
        print(self.board[6], self.board[7], self.board[8])
        print()

    def p1(self):
            try:
                print("PLAYER X")
                question = int(input("Type where your X should be placed\n"))
                if self.board[question] != "X" and self.board[question] != "O":
                    self.board[question] = "X"
                    self.create_board()
                    self.moves_count += 1
                else:
                    print("someone already took that row")
                    self.p1()
                self.check_win()
            except ValueError:
                print("Write a NUMBER from 0 to 8")
                self.p1()

    def p2(self):
        try:
            print("PLAYER 0")
            question = int(input("Type where your 0 should be placed\n"))
            if self.board[question] != "X" and self.board[question] != "O":
                self.board[question] = "O"
                self.create_board()
                self.moves_count += 1
            else:
                print("someone already took that row")
                self.p2()
            self.check_win()
        except:
            print("Write a NUMBER from 0 to 8")
            self.p2()

    def check_win(self):
        for a in self.win_conditions:
            if self.board[a[0]] == self.board[a[1]] == self.board[a[2]] == "X":
                print("PLAYER X WINS")
                self.play_again()
            if self.board[a[0]] == self.board[a[1]] == self.board[a[2]] == "O":
                print("PLAYER O WINS")
                self.play_again()
            elif self.moves_count == 9:
                print("A DRAW!")
                exit()

    def play(self):
        while True:
            self.p1()
            self.p2()

    def run(self):
        self.create_board()
        self.play()

    def play_again(self):
        while True:
            question = input("Do you want to play again? Type y or n\n")
            if question == "y":
                print("GLHF")
                self.board = [0, 1, 2, 3, 4, 5, 6, 7, 8]
                self.run()
            elif question == "n":
                print("See you next time!")
                quit()
            else:
                print("Thats not a valid option")

cos = Game()
cos.run()

Answer 1

Let's start with a bit of theory about what a class is, why you would use one, and what makes for a good one. The key idea about classes is that a class is a blueprint for a thing, and that blueprint says how that thing works. Within the class, you define all the nitty gritty details and write code carefully to keep those details nice and orderly, and from outside the class you are free to completely forget about it and assume it just works.

The first change I would suggest with your code is separate the class that represents the game from the code that deals with the players. After all, the game is one thing that you can draw a ring around in your head, and the players are outside that ring.

Now, the game should take care of details like remembering who has played where, where someone is still allowed to play, what the board will look like if someone plays in a given square, whether this board represents a game that has been won or drawn or such like, and so on. Meanwhile the players should take care of details like deciding where to play. For example in the Game class you might have

def is_move_legal(self, move):
    return self.board[move] != "X" and self.board[move] != "O"

And then the players, instead of

if self.board[question] != "X" and self.board[question] != "O":

Would have something like

if current_game.is_move_legal(question):

You can imagine how it's easier to work out what the second version is doing while you're thinking about how to design the players, because you don't have to think about what the board is like. As a bonus, it's easy to tell at a glance because the function has been named in a descriptive way.

Following this approach to designing a class would be quite an extensive rewrite of your game, but it would make it much clearer

---

As a general rule in programming, if you find yourself writing substantially the same code twice, it is worth thinking about whether you can do it once and make up the differences with a couple of variables. In particular in this case, the p1 and p2 functions are completely identical except for what letter they print and calling the other one. So you could have just one of them, and pass the "X" or the "O" it has to print and play with as a parameter.

---

It's important to name your functions precisely, so you can tell what they do. For example create_board would probably be better named print_board.

Similarly it's important to name your variables so that it's clear what they represent. You call both variables that you get from user input question. I can see why, but a name like chosen_move would be clearer. (But on the topic of user input, well done for checking user input, including in play_again.)

Referring to check_win, it is generally a big of a warning sign if you find yourself using single letter variable names. Think about what a represents, and call it that.

---

Now for a few pesky details, which are worth the time to polish.

The second player is "O" rather than "0" which you sometimes print.

In check_win are you sure you want to exit the game after a draw, but allow a rematch otherwise?

In play you have an infinite loop of switching between the two players. That's not an inherently bad thing. So long as you have some way of breaking out of it once the game has run its course, looping is a perfectly valid way to repeatedly let the same bits of code (the player choices) run. However, in this case it never even gets half way first the first loop, because p1 calls p2 directly, and that calls p1 and so on. This recursive approach is also a valid way to do that, but you should choose one and stick with it.

Answer 2

One more thing, when you play again self.moves_count needs to be reset to zero.

Answer 3

In 'check_win', the logic is missing the 9th move winning player validation. Your logic will state "A DRAW!" on 9th move without checking 8 'self.win_conditions'. If one player wins on the last move, it will say "A DRAW!". It appears that you are quite talented based on your code. I believe that you will find the answer easily without me laying out details of fix. Good job! I appreciate sharing your work.