First Attempt at Object-Oriented Programming: Tic-Tac-Toe

Question

I began learning python about 3 months ago, and this was my first OOP. I was hoping to get constructive feedback. My own thoughts after writing it were that there had to be a better way to: 1) generate the game board 2) code the logic for determining the winner via horizontal, vertical, or diagonal.

class Board:
 def __init__(self):
 self.positions = [None, ]
 for x in range(1,10):
 self.positions.append(str(x))
 def __getitem__(self, x):
 return self.positions[x]
 
 def generate_board(self):
 for x in range(2):
 if x == 0:
 print(self.positions[1].center(3,'_') + '|' + self.positions[2].center(3,'_') + '|' + self.positions[3].center(3,'_'))
 else:
 print(self.positions[4].center(3,'_') + '|' + self.positions[5].center(3,'_') + '|' + self.positions[6].center(3,'_'))
 print(self.positions[7].center(3) + '|' + self.positions[8].center(3) + '|' + self.positions[9].center(3))
class Player:
 def __init__(self, name, piece):
 self.name = name
 self.piece = piece
class Game:
 def __init__(self):
 name1 = input('p1 name: ')
 name2 = input('p2 name: ')
 p1_piece = input('Pick your piece player 1: (Type X or O) ')
 if p1_piece != 'X' or p1_piece != 'O':
 p1_piece = input('Player 1 did not pick a valid piece: (Type X or O) ')
 if p1_piece == 'X':
 p2_piece = 'O'
 else:
 p2_piece = 'X'
 self.p1 = Player(name1, p1_piece)
 self.p2 = Player(name2, p2_piece)
 self.positions = Board()
 self.win = False
 self.winner = ''
 def play_game(self):
 tie_count = 0
 while self.win == False:
 
 #generating board
 print('\n')
 print('Here is the board: ')
 Board.generate_board(self)
 #player 1's turn. Checking for win or tie in between turns.
 print('\n' + "Player 1's turn. Type the position where you would like to place your piece")
 position = input()
 if position in self.positions.positions:
 self.positions.positions[int(position)] = self.p1.piece
 print('\n')
 print('Here is the board: ')
 Board.generate_board(self)
 Game.game_check(self)
 if self.win == True:
 break
 #checking to see if the game is a tie. 
 for i in range(1,10):
 if str(i) not in self.positions:
 tie_count += 1
 if tie_count == 9:
 self.winner += "It's a tie! Nobody"
 self.win = True
 else:
 tie_count = 0
 if self.win == True:
 break
 print('\n' + "Player 2's turn. Type the position where you would like to place your piece")
 position = input()
 if position in self.positions.positions:
 self.positions.positions[int(position)] = self.p2.piece
 print('\n')
 print('Here is the board: ')
 Board.generate_board(self)
 Game.game_check(self)
 if self.win == True:
 break
 print('\n')
 print('The game is over! {} wins!'.format(self.winner))
 def game_check(self):
 #establishing counters for later use
 p1_bucket = 0
 p2_bucket = 0
 
 #below code counts horizontal wins
 for i in range(1,10,3):
 if self.positions.positions[i:i+3].count(self.p1.piece) == 3:
 self.win = True
 self.winner += self.p1.name
 elif self.positions.positions[i:i+3].count(self.p2.piece) == 3:
 self.win = True
 self.winner += self.p2.name
 
 #below code counts vertical wins
 for x in range(1,4):
 #nested for loop accomplishes sliding pattern for vertical combos (i.e. 1,4,7 then 2,5,8 etc.)
 for y in range(x,x+7,3):
 if self.positions.positions[y] == self.p1.piece:
 p1_bucket += 1
 elif self.positions.positions[y] == self.p2.piece:
 p2_bucket += 1
 if p1_bucket == 3: 
 self.win = True
 self.winner += self.p1.name
 break
 elif p2_bucket == 3:
 self.win = True
 self.winner += self.p2.name
 break
 else:
 p1_bucket = 0
 p2_bucket = 0
 #below code counts diagonal wins
 for x in range(1,10,4):
 if self.positions.positions[x] == self.p1.piece:
 p1_bucket += 1
 elif self.positions.positions[x] == self.p2.piece:
 p2_bucket += 1
 if p1_bucket == 3: 
 self.win = True
 self.winner += self.p1.name
 elif p2_bucket == 3:
 self.win = True
 self.winner += self.p2.name
 else:
 p1_bucket = 0
 p2_bucket = 0
 for x in range(3,8,2):
 if self.positions.positions[x] == self.p1.piece:
 p1_bucket += 1
 elif self.positions.positions[x] == self.p2.piece:
 p2_bucket += 1
 if p1_bucket == 3: 
 self.win = True
 self.winner += self.p1.name
 elif p2_bucket == 3:
 self.win = True
 self.winner += self.p2.name
 else:
 p1_bucket = 0
 p2_bucket = 0
game1 = Game()
game1.play_game()

Answer 1

Bug #1

Player 1 is always initially denied when picking the piece because this condition is never true:

if p1_piece != 'X' or p1_piece != 'O':

The player eventually gets a pick, but this is not what was intended. The logic is incorrect. One way to fix it is to replace or with and:

if p1_piece != 'X' and p1_piece != 'O':

A better way is to use:

 p1_piece = input('Pick your piece player 1: (Type X or O) ').upper()
 if p1_piece == 'X':
 p2_piece = 'O'
 else:
 p1_piece = 'O'
 p2_piece = 'X'

Bug #2

Player 2 is allowed to pick the same square the Player 1 picked. This should not be allowed. The code should remember all squares that are occupied and force the next player to keep guessing until an empty square is guessed.

Simplify

In the generate_board function, there is no need for the for loop or the if/else:

 for x in range(2):
 if x == 0:

You just need the print statements:

def generate_board(self):
 print(self.positions[1].center(3,'_') + '|' + self.positions[2].center(3,'_') + '|' + self.positions[3].center(3,'_'))
 print(self.positions[4].center(3,'_') + '|' + self.positions[5].center(3,'_') + '|' + self.positions[6].center(3,'_'))
 print(self.positions[7].center(3) + '|' + self.positions[8].center(3) + '|' + self.positions[9].center(3))

This:

 while self.win == False:

is more commonly written as:

 while not self.win:

This:

 if self.win == True:

is more commonly written as:

 if self.win:

DRY

Yes, there is a lot of repeated code on the row/column/diagonal checks.

The 2 diagonal checks can be combined into a new function:

def game_diag(self, start_square):
 """
 Check a diagonal.
 start_square: [1|3] This is the starting square of a diagonal in the top row.
 """
 p1_bucket = 0
 p2_bucket = 0
 if start_square == 1:
 squares = (1, 5, 9)
 else:
 squares = (3, 5, 7)
 for x in squares:
 if self.positions.positions[x] == self.p1.piece:
 p1_bucket += 1
 elif self.positions.positions[x] == self.p2.piece:
 p2_bucket += 1
 if p1_bucket == 3: 
 self.win = True
 self.winner += self.p1.name
 elif p2_bucket == 3:
 self.win = True
 self.winner += self.p2.name

Then the function is called from game_check:

 #below code counts diagonal wins
 self.game_diag(1)
 if self.win:
 break
 self.game_diag(3)

Note that I added a docstring for the game_diag function to describe its purpose. The PEP 8 style guide recommends docstrings for classes and functions.