4
\$\begingroup\$

I was looking at a code review of a Tic Tac Toe game in Python when I recalled a talk about not using classes, and I wanted to see if I could write a readable, working implementation of the game without classes.

In order, I am interested in improving the code's:

  1. readability,
  2. simplicity,
  3. idiomatic ("Pythonic") usage.

I think there's probably room for improvement around the print statements (stars feels clunky to me) and improving the readability around the code using potential_win_sequences, but it's not immediately obvious how to improve it.

import itertools

import numpy as np

players = ["X", "O"]
numbered_positions = np.array([str(i) for i in range(1, 10)]).reshape(3, 3)

# Start a clean board.
board = np.array(["?"] * 9).reshape(3, 3)

# Take turns.
for player in itertools.cycle(players):
    valid_choice = False
    while valid_choice is False:
        print(np.where(board == "?", numbered_positions, board), "\n")

        print(f"{player}: Pick a spot [1-9]: ")
        user_input = input()
        try:
            spot = int(user_input)
            if spot < 1 or spot > 9:
                raise ValueError
            spot_index = spot - 1
        except ValueError:
            error_msg = "Invalid entry, please enter a number 1-9."
            stars = "*" * len(error_msg)
            print(stars, error_msg, stars, sep="\n")
        else:
            if board.ravel()[spot_index] == "?":
                valid_choice = True
                board.ravel()[spot_index] = player
            else:
                error_msg = "Choose an empty spot."
                stars = "*" * len(error_msg)
                print(stars, error_msg, stars, sep="\n")

    # Check for end of game.
    potential_win_sequences = np.vstack(
        [
            board,                        # Check rows.
            board.T,                      # Check columns.
            board.diagonal(),             # Check diagonal.
            np.fliplr(board).diagonal(),  # Check antidiagonal.
        ]
    )

    if any((i == player).all() for i in potential_win_sequences):
        print(board)
        print(f"{player} wins!")
        break
    elif "?" not in board:
        print(board)
        print("Tie game.")
        break
\$\endgroup\$
3
\$\begingroup\$

Readability

Break the code up into multiple functions, that each do their own small task. The main loop should read like:

for player in itertools.cycle(players):
     spot = pick_spot(board)
     place_mark(board, player, spot)
     print(board)

     if (check_win_condition(board)):
         print(f"{player} wins!")
         break
     elif (check_tie_condition(board)):
         print("Tie game.")
         break

Simplicity

Yes, stars is clunky, but also unnecessary. Keep it simple, and just don't print stars around error messages! If you want to make an error message stand out, just start the line with the text ERROR:.

Idiomatic code

It looks quite idiomatic to me. You're leaning heavily on NumPy, so it's more idiomatic NumPy code than pure Python code I would say.

Combine numbered_positions and board

Why not have numbered_positions be the initial board, and every time a player moves, you replace a number by X or O? This removes a variable and makes printing the board easier, although it would make checking for a tie game a little more complex.

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

This is not a complete answer, just a small point:

while valid_choice is False:

is more Pythonic written as

while not valid_user_input:
| improve this answer | |
\$\endgroup\$

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.