“Tic Tac Toe” game in Python

Question

I'm relatively new to Python. Here is a Tic Tac Toe game I've created in Python:

def is_valid_move(x,y,board):
    return ((0 <= x <= 2) and (0 <= y <= 2)) and (board[x][y] == '_')

def check_win(board):
    if board[0][0] == board[0][1] == board[0][2] != '_':
        return True
    if board[1][0] == board[1][1] == board[1][2] != '_':
        return True
    if board[2][0] == board[2][1] == board[2][2] != '_':
        return True
    if board[0][0] == board[0][1] == board[0][2] != '_':
        return True
    if board[0][1] == board[1][1] == board[2][1] != '_':
        return True
    if board[0][2] == board[1][2] == board[2][2] != '_':
        return True
    if board[0][0] == board[1][1] == board[2][2] != '_':
        return True
    if board[2][0] == board[1][1] == board[0][2] != '_':
        return True
    return False

def get_input_from_user(message:str):
    coord = input(message)
    str_x, str_y = coord.split()
    return int(str_x), int(str_y)


def make_move(idx,board):
    if idx % 2 == 0:
        x, y = get_input_from_user("Player X, please enter a move:")
        while not is_valid_move(x, y,board):
            x,y = get_input_from_user("Invalid move, please try again:")
        board[x][y] = 'X'
    else:
        x, y = get_input_from_user("Player O, please enter a move:")
        while not is_valid_move(x, y,board):
            x, y = get_input_from_user("Invalid move, please try again:")
        board[x][y] = 'O'

def print_board(board):
    for i in range(3):
        print(board[i])

if __name__ == '__main__':
    idx = 0
    board = [['_','_','_'] for i in range(3)]
    condition = False
    while condition is not True or idx <= 8:
        make_move(idx,board)
        print_board(board)
        condition = check_win(board)
        if condition is True and idx % 2 == 0:
            print("Player X is the Winner!")
        elif condition is True:
            print("Player O is the Winner!")
        idx += 1
    if idx > 8 and condition is not True:
        print("Its a Draw!")

Here is a simple test run of this code:

Player X, please enter a move:0 2
['_', '_', 'X']
['_', '_', '_']
['_', '_', '_']
Player O, please enter a move:0 1
['_', 'O', 'X']
['_', '_', '_']
['_', '_', '_']
Player X, please enter a move:1 1
['_', 'O', 'X']
['_', 'X', '_']
['_', '_', '_']
Player O, please enter a move:0 0
['O', 'O', 'X']
['_', 'X', '_']
['_', '_', '_']
Player X, please enter a move:2 0
['O', 'O', 'X']
['_', 'X', '_']
['X', '_', '_']
Player X is the Winner!

Let me know your thoughts about my code.

Answer 1

I see a few areas where you can improve.

• You don't need parenthesis around the expressions you and or return:

  def is_valid_move(x, y, board):
      return 0 <= x <= 2 and 0 <= y <= 2 and board[x][y] == '_'
  

  Sometimes you do need parenthesis, but only if the operator precedence is wrong otherwise. I.e A and B or C is different from A and (B or C).

• Your check_win function is very hard to debug. How sure are you that you did not mess up any of the indices while copy&pasting, and how long would it take you to check that you didn't?

  Instead divide the function into sub-responsibilities, which you can name and reason about individually:

  def all_equal(it):
      it = iter(it)
      first = next(it)
      return all(x == first for x in it)
  
  def check_rows(board):
      for row in board:
          if row[0] != "_" and all_equal(row):
              return True
      return False
  
  def check_cols(board):
      for col in zip(*board):  # transpose the board
          if col[0] != "_" and all_equal(col):
              return True
      return False
  
  def check_diagonals(board):
      if board[0][0] != "_" and all_equal(board[i][i] for i in range(len(board))):
          return True
      if board[0][2] != "_" and all_equal([board[0][2], board[1][1], board[2][0]]):
          return True
      return False
  
  def check_win(board):
      return check_rows(board) or check_cols(board) or check_diagonals(board)
  

  While this code is longer, it should also be more readable. Also, this way only the diagonal check is still complicated to check for correctness, but at least it is localized.

• Your get_input_from_user function can be shortened a bit, at the cost of some readability (at least until you get used to functional programming):

  def get_input_from_user(message: str):
      return tuple(map(int, input(message).split()))
  

• Don't iterate over the indices, if what you really want to do is iterate over the elements:

  def print_board(board):
      for row in board:
          print(row)
  

• Your main loop and the make_move function. The make_move function has two almost identical branches, the only difference is the player mark. So why not pass that instead of the index? In your main loop you can also change that to just swap between the two players, which lets you use a for loop which is automatically restricted to n iterations:

  def make_move(player, board):
      x, y = get_input_from_user(f"Player {player}, please enter a move:")
      while not is_valid_move(x, y, board):
          x,y = get_input_from_user("Invalid move, please try again:")
      board[x][y] = player
  
  if __name__ == '__main__':
      players = "X", "O"
      board = [['_'] * 3 for _ in range(3)]
      for _ in range(9):
          make_move(players[0], board)
          print_board(board)
          if check_win(board):
              print(f"Player {players[0]} is the Winner!")
              break
          players = players[::-1]  # equivalent to players[0], players[1] = players[1], players[0]
      else:    # only runs if no `break` broke the loop
          print("Its a Draw!")
  

  I also used some f-strings for formatting, multiplication of lists, and the else clause of loops to detect a draw.

• Python has an official style-guide, PEP8. It recommends always puting a space after commas in parameter lists, etc.

Answer 2

This code looks quite good! Just some recommendations:

---

Try to follow the PEP 8 Style Guidelines so that your code is more standard and readable; you already do it with the naming, but you are missing some whitespaces: is_valid_move(x,y,board) should be is_valid_move(x, y, board), (message:str) should be (message: str),...

---

In the make_move function you are repeating the same code twice: the only thing that changes is the text displayed and the symbol used. In fact, the prompt text just depends on the player symbol. So it can be rewritten as this:

def make_move(idx,board):
    if idx % 2 == 0:
        symbol = 'X'
    else:
        symbol = 'O'

    x, y = get_input_from_user(f"Player {symbol}, please enter a move:")
    while not is_valid_move(x, y, board):
        x,y = get_input_from_user("Invalid move, please try again:")
    board[x][y] = symbol

Here I used an f-string to format the prompt text. If you want shorter code, you can also replace the conditional with a ternary:

symbol = 'X' if idx % 2 == 0 else 'O'

---

The print_board can be made more contat using a "for each" loop. Instead of iterating indices, you iterate the elements of the list directly:

for row in board:
    print(row)

---

board = [['_','_','_'] for i in range(3)]

When the index variable in a loop is not used, it's better to use the anonymous _ instead. Furthermore, you can use * to make a list with a default value:

board = [['_'] * 3 for _ in range(3)

It might be tempting to just do [['_'] * 3] * 3, but that's a trap: board will contain the same list reference for all three rows, so editing one will edit all of them.

---

Variable names should clearly indicate what they are; condition does not seem clear enough to me, as I had to look at the code to understand its meaning. Try a more explicit name like has_winner.

Additionally, condition is a boolean (can only be true or false), so you don't have to check is True explicitly; you can just do if condition and ... (use if not condition to check if it's false).