7
\$\begingroup\$

I've been teaching myself Python and decided to make a Tic Tac Toe game for a bit of practice. Any criticisms or pointers are welcome!

class TicTacToe:
  """A class for playing tic tac toe"""

  def __init__(self):
    self.values = ["-", "-", "-", 
                  "-", "-", "-", 
                  "-", "-", "-"]
    self.player = "x"


  def show_board(self):
    print(f"{self.values[0]} | {self.values[1]} | {self.values[2]}")
    print(f"{self.values[3]} | {self.values[4]} | {self.values[5]}")
    print(f"{self.values[6]} | {self.values[7]} | {self.values[8]}")


  def play_game(self):
    # display initial board
    self.show_board()
    # play for 9 turns max
    for i in range(9):
      self.handle_turn(self.values)
      if self.check_tie() or self.winner():
        break


  def handle_turn(self, values):
    # get next move from a player
    try:
      turn = int(input(f"Player {self.player} pick a square (1-9) from left to right: "))
    except ValueError:
      self.handle_turn(self.values)
      return
    
    # change values to show the move made (check square empty)
    if self.values[turn-1] == "x" or self.values[turn-1] == "o":
      print("That square has been played already!")
      self.handle_turn(self.values)
    else:
      self.values[turn-1] = self.player
      self.show_board()
    # switch turns
    self.flip_player()


  def winner(self):
    # check all possible win methods
    self.row_winner = self.check_rows()
    self.column_winner = self.check_columns()
    self.diagonal_winner = self.check_diagonals()
    #declare a winner
    if self.row_winner:
      winner = self.check_rows()
      print(f"{winner} has won the game!")
      return True
    elif self.column_winner:
      winner = self.check_columns()
      print(f"{winner} has won the game!")
      return True
    elif self.diagonal_winner:
      winner = self.check_diagonals()
      print(f"{winner} has won the game!")
      return True


  def check_rows(self):
    # Check for a win in the rows
    row1 = self.values[0] == self.values[1] == self.values[2] != "-"
    row2 = self.values[3] == self.values[4] == self.values[5] != "-"
    row3 = self.values[6] == self.values[7] == self.values[8] != "-"
    # Return the player that has won
    if row1:
      return self.values[0]
    elif row2:
      return self.values[3]
    elif row3:
      return self.values[6]  
  

  def check_columns(self):
    # Check for a win in the columns
    col1 = self.values[0] == self.values[3] == self.values[6] != "-"
    col2 = self.values[1] == self.values[4] == self.values[7] != "-"
    col3 = self.values[2] == self.values[5] == self.values[8] != "-"
    # Return the winning player
    if col1:
      return self.values[0]
    elif col2:
      return self.values[1]
    elif col3:
      return self.values[2]


  def check_diagonals(self):
    # Check for win in the diagonals
    dia1 = self.values[0] == self.values[4] == self.values[8] != "-"
    dia2 = self.values[2] == self.values[4] == self.values[6] != "-"
    # Return the player that has won
    if dia1:
      return self.values[0]
    elif dia2:
      return self.values[2]


  def check_tie(self):
    if "-" not in self.values:
      print("Game is a tie!")
      return True
    else:
      return False


  def flip_player(self):
    if self.player == "x":
      self.player = "o"
    else:
      self.player = "x"

if __name__ == "__main__":
  new_board = TicTacToe()
  new_board.play_game()
\$\endgroup\$
1
  • 2
    \$\begingroup\$ have you missed a else: return false in winner() ? Relying on the null value is perhaps not advisable \$\endgroup\$ – Hannah W. May 3 at 20:43
4
\$\begingroup\$

Visuals

You can try

self.values = [["-" for _ in range(3)] for _ in range(3)]

This will let you set up a 3x3 grid without having to draw the whole thing.

Now you can also do

    def show_board(self):
        board_str = "\n".join([ " | ".join(row) for row in grid ])
        print(board_str)

I would also suggest doing away with the show_board function altogether and instead use (for printing)

    def __str__(self):
        return "\n".join([ " | ".join(row) for row in grid ])

and/or (for printing in interactive console)

    def __repr__(self):
        return "\n".join([ " | ".join(row) for row in grid ])

Then you can directly do

xoxo = TicTacToe
print(xoxo)

and inside the class

print(self)

Data Representation and Input

I'm repeating myself here, but it doesn't make sense to have values be a 1-dimensional array. It's a tic-tac-toe board... it should be a 2D 3x3 grid or board. Also you should probably rename the variable values to grid or board.

This will mean taking 2 values as input from user in handle_turn(). I suggest not putting a large string inside input() as it's hard to read. Try instead

print("Enter space separated co-ordinates to pick a square")
print(f"Player {self.player} pick a square: "))
x = int(input())
y = int(input())

Ok, I'll admit this is a little annoying for the players.... maybe you could write the coordinates in the empty cells like

 X  | 1,2 |  O
2,1 |  X  | 1,3
3,1 | 3,2 |  O

(try to do this without writing out the whole starting grid manually, it should be possible using ",".join(), str() and list comprehensions)

Also it should be possible to modify the row/column/diagonals functions with 2D indices like grid[0][0], grid[1][1], grid[2][2], etc. to make it more readable. Due to the way self.grid is stored, the row function can be simplified... but it will look very different from column/diagonal and so it's probably not worth it.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ This is a useful review, and for beginners implementing tic-tac-toe with a 2d grid is intuitive -- so your advice is sound. That said, I have seen job candidates implement tic-tac-toe solutions in efficient, readable ways using 1d list (diagonal checks are easier in flatland, for example; same can arguably be said for collecting user input). Anyway, not a big deal, but it can be reasonable to solve grid problems with 1d data behind the scenes. \$\endgroup\$ – FMc May 3 at 23:39
  • 1
    \$\begingroup\$ @Hannah W. When using self.values = [ ["-"] * 3 ] * 3 it seems like a copy of the original list is made three times, as in all three rows will be changed when making a single move, I may have implemented this incorrectly. Could it be better practice to use something like self.values = [["-"] * 3 for _ in range(3)] instead? \$\endgroup\$ – JLByrne May 4 at 13:11
  • 1
    \$\begingroup\$ @JLByrne yea, I'd missed this. Actually, it would be better to not mix syntax and just stick with 2 nested list comprehension imo. \$\endgroup\$ – Hannah W. May 4 at 14:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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