Skip to content

Possum - Makiko Y.#18

Open
anselrognlie wants to merge 5 commits intoAda-C24:mainfrom
trimakichan:main
Open

Possum - Makiko Y.#18
anselrognlie wants to merge 5 commits intoAda-C24:mainfrom
trimakichan:main

Conversation

@anselrognlie
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown
Author

@anselrognlie anselrognlie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Please review my comments, and let me know if you have any questions. Nice job!

Comment thread game.py
Comment on lines +25 to +28
# Track which letters in the word have been guessed correctly.
correct_letter_guess_statuses = build_letter_status_dict(snowman_word)
# Track letters the player guessed letters are not in the word.
wrong_guesses_list = []
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 These are the two pieces of data we need to make use of our other helper functions for tracking correct and incorrect guesses. Tracking the letter status involves using a dictionary with a particular structure, which our build_letter_status_dict helper is able to calculate from the random word.

Comment thread game.py Outdated
Comment on lines +31 to +34
while (
len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES
and not is_word_guessed(snowman_word, correct_letter_guess_statuses)
):
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Nice inclusion of both key checks for whether the game is still in progress. The game continues as long as the allowed number of wrong guesses haven't been used up, and the word hasn't yet been guessed. We had to be careful to express this with "as long as" phrasing, which is a tricky part of working with while loops since people often think of doing something "until" a condition is met rather than "as long as" a condition continues to hold.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, we could shorten this condition to focus solely on the number of guesses, and have a check within the loop for whether the word has been guessed.

    while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES:

        # later
        if is_word_guessed(snowman_word, correct_letter_guess_statuses):
            print("Congratulations, you win!")
            print_word_progress_string(snowman_word, correct_letter_guess_statuses)
            return

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With respect to the indentation, check out this section of PEP8 for some ideas. As written, this feels a bit spread out to me. I might write it as

    while (len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES
            and not is_word_guessed(snowman_word, correct_letter_guess_statuses)):
        # loop body

The extra indentation on the wrapped condition helps keep it separate from the following loop body. Opinions on this topic are definitely varied, though.

Comment thread game.py
len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES
and not is_word_guessed(snowman_word, correct_letter_guess_statuses)
):
user_input = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we don't display the word status until after the guess, the player doesn't get to see the length of the word before guessing. This wasn't a requirement, but it would be nice to have from a gameplay perspective.

Comment thread game.py Outdated
):
user_input = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list)

if user_input in snowman_word:
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking that the guessed letter is in the word is a valid way to check whether this was a correct letter. We haven't discussed this yet, but checking whether a letter is in a string requires Python to check each letter one by one, while checking whether a letter is the key of a dict (also performed with the in operator) can theoretically be performed with a single computation. It won't matter much in this small program, but it's helpful to start thinking about these differences and biasing our choices towards efficiency!

Comment thread game.py
else:
print(f"\nYou guessed it wrong. The lettter, {user_input} is NOT in the word!\n")
wrong_guesses_list.append(user_input)
print(f"You have {SNOWMAN_MAX_WRONG_GUESSES - len(wrong_guesses_list)} tries left.\n")
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Showing the count of remaining guesses is a nice touch. It can be tough to tell how many guesses remain just from the snowman drawing.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Nice decision to get the count of wrong guesses directly from the length of the wrong guesses list when needed. If we had used a separate variable to track the count, it would be possible for the values to become out of sync, leading to player confusion. Though unlikely in a program this short, in longer programs it can be more difficult to be sure that related values are always updated together. Forgetting to update both in one place can lead to them drifting.

In some languages, getting the length of certain data types is a relatively slow operation. Python is fast for most built-in containers (and strings), so it's fine to call len() as many times as we need to. But we do need to understand the costs of operations in the languages we are working in. In a language where getting the length of a container might be costly (or even impossible), it might be worthwhile (or necessary) to track the count in a separate variable.

Comment thread game.py Outdated
# Print snowman, word progress and wrong guesses after each turn.
print_snowman_graphic(len(wrong_guesses_list))
print_word_progress_string(snowman_word, correct_letter_guess_statuses)
print(f"\nWrong guesses: {wrong_guesses_list}\n")
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python is able to print out a list of strings, which lets us show the wrong guesses to the player. However, we might consider writing our own function to customize the string we build (try looking into the join method of the string type) since the default output is somewhat "programmer-y." A typical player doesn't need to see the surrounding [] nor the '' around each character.

Comment thread game.py Outdated
print("=====================================")

# End message.
if is_word_guessed(snowman_word, correct_letter_guess_statuses):
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Our loop terminates either because the number of wrong guesses was used up, or because the word has been guessed. As a result, we do need to double check whether the game was won or lost. With this structure, it would be just as valid to check whether we were out of guesses, meaning the player lost, otherwise they won.

Another alternative would be to drive the main loop using only one of the conditions (either there are still wrong guesses available, OR that the word hasn't yet been guessed). Then within the loop, we could check for the other condition and handle it directly. For example, if we ran the main loop based on the number of wrong guesses alone, we could have an if check within the loop checking for the word being guessed. If we find that the word has been guessed, we could print out the winning message, then return from the entire function. This would then mean that if we exited the loop normally, it must have been because the player lost, so we could print the lose message without needing to double check whether the player won or lost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants