Chomp Game in Python 3

Question

I have deliberately avoided any OOP for this project. I'm fairly happy with my implementation of the classic strategy game "Chomp". The computer just uses a random valid choice, so no AI.

I would appreciate feedback on points of style and correctness. Is the code basically OK? Does it seem to have a coherent style? What improvements would you make?

Thanks in advance.

"""
Chomp - a strategy game
"""
import random
import time
NUM_ROWS = 5
NUM_COLS = 6
def print_title():
 print(r"""
 ______ __ __ ______ __ __ ______ 
/\ ___\ /\ \_\ \ /\ __ \ /\ "-./ \ /\ == \ 
\ \ \____ \ \ __ \ \ \ \/\ \ \ \ \-./\ \ \ \ _-/ 
 \ \_____\ \ \_\ \_\ \ \_____\ \ \_\ \ \_\ \ \_\ 
 \/_____/ \/_/\/_/ \/_____/ \/_/ \/_/ \/_/ 
""")
def print_instructions():
 print("Welcome to Chomp. Choose a square and all squares to the right")
 print("and downwards will be eaten. The computer will do the same.")
 print("The one to eat the poison square loses. Good luck!")
 print()
def who_goes_first():
 if random.randint(0, 1) == 0:
 return "computer"
 else:
 return "human"
def play_again():
 print("Would you like to play again? (yes or no)")
 return input().lower().startswith("y")
def print_matrix(matrix):
 for row in matrix:
 for elem in row:
 print(elem, end=" ")
 print()
def validate_user_input(player_choice, board):
 try:
 row, col = player_choice.split()
 except ValueError:
 print("Bad input: The input should be exactly two numbers separated by a space.")
 return False
 try:
 row = int(row)
 col = int(col)
 except ValueError:
 print("Input must be two numbers, however non-digit characters were received.")
 return False
 if row < 0 or row > NUM_ROWS - 1:
 print(f"The first number must be between 0 and {NUM_ROWS - 1} but {row} was passed.")
 return False
 if col < 0 or col > NUM_COLS - 1:
 print(f"The second number must be between 0 and {NUM_COLS - 1} but {col} was passed.")
 return False
 if board[row][col] == " ":
 print("That square has already been eaten!")
 return False
 return True
def update_board(board, row, col):
 for i in range(row, len(board)):
 for j in range(col, len(board[i])):
 board[i][j] = " "
def get_human_move(board):
 valid_input = False
 while not valid_input:
 player_choice = input("Enter the row and column of your choice separated by a space: ")
 valid_input = validate_user_input(player_choice, board)
 row, col = player_choice.split()
 return int(row), int(col)
def get_computer_move(board):
 valid_move = False
 while not valid_move:
 row = random.randint(0, NUM_ROWS - 1)
 col = random.randint(0, NUM_COLS - 1)
 if board[row][col] == " ":
 continue
 else:
 valid_move = True
 return row, col
def main():
 board = []
 for i in range(NUM_ROWS):
 row = []
 for j in range(NUM_COLS):
 row.append("#")
 board.append(row)
 board[0][0] = "P"
 game_is_playing = True
 turn = "human"
 print_title()
 print_instructions()
 while game_is_playing:
 if turn == "human":
 # Human turn
 print("Human turn.")
 print()
 print_matrix(board)
 print()
 row, col = get_human_move(board)
 if board[row][col] == "P":
 print()
 print("Too bad, the computer wins!")
 game_is_playing = False
 else:
 update_board(board, row, col)
 print()
 print_matrix(board)
 print()
 turn = "computer"
 time.sleep(1)
 else:
 # Computer turn
 row, col = get_computer_move(board)
 print(f"Computer turn. the computer chooses ({row}, {col})")
 print()
 if board[row][col] == "P":
 print()
 print("Yay, you win!")
 game_is_playing = False
 else:
 update_board(board, row, col)
 print_matrix(board)
 print()
 turn = "human"
 if play_again():
 main()
 else:
 print("Goodbye!")
 raise SystemExit
main()

• Mast: Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers .
• Robin Andrews: I was just removing "clutter" as the big ASCII title is superfluous. I didn't change anything else. I would prefer the simplified title.
• Mast: The title is explicitly mentioned in an existing answer, so we can't do that.

Answer 1

Your game has the same kind of flaw like a lot of the other games that have recently been here on Code Review: an unnecessary recursion in the main game flow:

What do I mean by that? Let's look at your main function:

def main(): 
 # ... all of the actual game here ...
 if play_again():
 main()
 else:
 print("Goodbye!")
 raise SystemExit

As one can clearly see main will call main every time the player chooses to play another round, leading to deeper and deeper recursion. If someone would be really obsessed with your game and would try to play more than sys.getrecursionlimit() games (here on my machine it's 3000), there would be a RuntimeError.

Fortunately this can easily be fixed using a simple while loop:

def main():
 while True:
 # ... all of the actual game here ...
 if not play_again():
 print("Goodbye!")
 break

Recursion gone, welcome binge playing till your fingers bleed.

---

Now that we have that sorted out, let's look at some of the details of your code.

print_*

Not much to critique here, but I would like to introduce you to textwrap.dedent. textwrap.dedent allows you to indent text blocks as you would do with code and then takes care to remove the additional leading whitespace.

from textwrap import dedent
def print_title():
 print(
 dedent(r"""
 ______ __ __ ______ __ __ ______ 
 /\ ___\ /\ \_\ \ /\ __ \ /\ "-./ \ /\ == \ 
 \ \ \____ \ \ __ \ \ \ \/\ \ \ \ \-./\ \ \ \ _-/ 
 \ \_____\ \ \_\ \_\ \ \_____\ \ \_\ \ \_\ \ \_\ 
 \/_____/ \/_/\/_/ \/_____/ \/_/ \/_/ \/_/ 
 """)
 )

But it basically boils down to personal taste which version you prefer. The same logic could be applied to print_instructions to only need a single call to print.

who_goes_first

who_goes_first could be simplified a little bit:

def who_goes_first():
 return random.choice(("computer", "human"))

Or even be left out? You current code does not use it.

print_matrix

Again, the code could be simplified a little bit, e.g. using str.join:

def print_matrix(matrix):
 for row in matrix:
 print(" ".join(row))

From what you can actually see, there should be no difference to the previous output, but there is actually no trailig whitespace in this version. You could even go a little bit futher than this using a list comprehension:

def print_matrix(matrix):
 print("\n".join(" ".join(row) for row in matrix))

validate_user_input / get_human_move

validate_user_input does a good job at validating the given user input, but keeps the results of the parsing to itself. That leads to duplicate code in get_human_move. With a little bit of rewriting that duplication can be removed:

def validate_user_input(player_choice, board):
 try:
 row, col = player_choice.split()
 except ValueError:
 raise ValueError(
 "Bad input: The input should be exactly two numbers separated by a space."
 )
 try:
 row = int(row)
 col = int(col)
 except ValueError:
 raise ValueError(
 "Input must be two numbers, however non-digit characters were received."
 )
 if row < 0 or row > NUM_ROWS - 1:
 raise ValueError(
 f"The first number must be between 0 and {NUM_ROWS - 1} but {row} was passed."
 )
 if col < 0 or col > NUM_COLS - 1:
 raise ValueError(
 f"The second number must be between 0 and {NUM_COLS - 1} but {col} was passed."
 )
 if board[row][col] == " ":
 raise ValueError("That square has already been eaten!")
 return row, col
def get_human_move(board):
 while True:
 player_choice = input("Enter the row and column of your choice separated by a space: ")
 try:
 row, col = validate_user_input(player_choice, board)
 break
 except ValueError as ex:
 print(ex)
 return row, col

So what has happened here?

1. validate_user_input now does not print the error message itself, but raises an informative exception instead.
2. if no reason to raise an exception has occured, validate_user_input now returns row and col, to they do not need to be recomputed in get_human_move
3. get_human_move was adapted to that change and now tries to get the validated user input, prints the reason if that fails, and asks the user to try again.

Of course, raising exceptions is just one way to do this. There are many other ways that lead to a similar structure.

If you decide to implement these changes, parse_user_input is maybe a more appropiate name now, given its new capabilites.

You might want think about passing NUM_ROWS/NUM_COLS as parameters or determine it from board to cut down on global variables as well.

Quick sidenote: 0-based indexing might be unfamiliar to people that have no programming backgroud (or use MatLab all the time ;-)), so maybe allow the user to enter 1-based indices and substract 1 before validation.

update_board

Depending on your choice on the previous point, this is either the way to go, or you should use NUM_ROWS/NUM_COLS here too to be consistent.

get_computer_move

Maybe you shoud add some simple heuristics to make your computer a little bit smarter, e.g. it sometimes chooses to take the poison even if it still has alternatives, or even the possibilty to win. Also (theoretically), it is be possible that this random sampling algorithm never finds a valid solution ;-) To avoid that, generate a list of valid rows and values, and pick a sample from these lists.

If you stick to the current approach, you can at least get rid of the "support variable" valid_move:

def get_computer_move(board):
 while True:
 row = random.randint(0, NUM_ROWS - 1)
 col = random.randint(0, NUM_COLS - 1)
 if board[row][col] == EMPTY_SPOT:
 continue
 else:
 break
 return row, col

main

main already had some structural remarks, so let's look at its code a little bit:

This

board = []
for i in range(NUM_ROWS):
 row = []
 for j in range(NUM_COLS):
 row.append("#")
 board.append(row)

can be recuded to a single nested list comprehension:

board = [["#" for _ in range(NUM_COLS)] for _ in range(NUM_ROWS)]

Depending on your choice regarding the global variables, NUM_ROWS/NUM_COLS would need to be removed here to. Maybe even allow them to be set as command line arguments by the user?

The rest of the code has some "magic values" (they are not so magic in your case), e.g. board[0][0] = "P", turn = "computer", and turn = "human" (there was also " " earlier to mark empty spots). The problem with those "magic values" is that your IDE has no chance to help you spot errors. You did write "p" instead of "P" and now the game does weird things? Too bad. You will have to find that bug yourself! The way to go about this would be to use global level constants, because this is what globals are actually good for, or an enum if you have several distint values like human and computer.

This is a possible way to do it (with a sketch of how main would look like):

FILLED_SPOT = "#"
POISON_SPOT = "P"
EMPTY_SPOT = " "
class Actor(enum.Enum):
 HUMAN = "human"
 COMPUTER = "computer"
# ... lot of code here ...
def main():
 while True:
 board = [[FILLED_SPOT for _ in range(NUM_COLS)] for _ in range(NUM_ROWS)]
 board[0][0] = POISON_SPOT
 turn = Actor.HUMAN # or: who_goes_first()
 # ...
 while game_is_playing:
 if turn == Actor.HUMAN:
 # Human turn
 # ...
 if board[row][col] == POISON_SPOT:
 # ...
 else:
 # ...
 turn = Actor.COMPUTER
 else:
 # Computer turn
 # ...
 if board[row][col] == POISON_SPOT:
 # ...
 else:
 # ...
 turn = Actor.HUMAN
 if not play_again():
 print("Goodbye!")
 break

I chose this to showcase both variants. In your case to could also just use either of those, no need to have both of them in the game.

---

All of this may sound harsh, but it really isn't meant that way! Your code is actually quite enjoyable to review. Your style is quite consistent, the names are good and the general structure is clear.

Keep up the good work!

• Robin Andrews: Thanks @AlexV That's exactly the kind of feedback I was looking for. One thing that confuses me though - I hear strong opinions about not using while True - that the condition should be made explicit, and yet it seems like a useful construct to me, and you are recommending it here. I'm guessing other reviewers would say to change it. Is it just a matter of taste?
• AlexV: I don't have a strong opinion on that. But I think I prefer the while True: ... break approach for simple cases like here, simply because that helps me not to have to find another meaningful name for a variable. If you have to check multiple conditions, or the inner loop would also need to break the outer loop, the explicit approach is the way to go I think.
• Robin Andrews: I've just been revisiting this code, and noticed something weird with global variables. For example in update_board I pass board as a parameter, and update it inside the function, but don't return the updated board, yet it still gets updated. So it appears I'm basically using a global value for board. Should I remove the board parameter and use global board instead, to be at least coherent? I know globals are discouraged, but it's maybe at least better to be explicit about them.
• AlexV: @Robin: Python has mutable (e.g. list, dict) and immutable types/objects (e.g. int, string, tuple) types. board is a list of list and therefore mutable, so you can modify it directly without returning the modified version. I don't see how a global variable would make this more obvious.