Possum - Priscilla A.#21
Conversation
anselrognlie
left a comment
There was a problem hiding this comment.
Looks good! Please review my comments, and let me know if you have any questions. Nice job!
| correct_letter_guess_statuses = build_letter_status_dict(snowman_word) | ||
| wrong_guesses_list = [] |
There was a problem hiding this comment.
👍 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.
| pass | ||
| correct_letter_guess_statuses = build_letter_status_dict(snowman_word) | ||
| wrong_guesses_list = [] | ||
| wrong_guesses_count = 0 |
There was a problem hiding this comment.
This variable to track the number of wrong guesses isn't necessary. Any time we need to know the count, we can get the length of the wrong guesses list. While there is a small bit of efficiency gained by not needing to call len(), the danger (not so much here, but in more complex code) is that we could update one of the list or the count in one location, but forget to update the other, meaning they are now out of sync.
So unless calculating a value from data we're already tracking has a high cost (we'll be talking more about this in future material), try to minimize introducing additional variables that duplicate information.
There was a problem hiding this comment.
Thank you, I agree with you, but I could only change the snowman function body, and the other helper methods used the variable wrong_guesses_count to print the snowman, so I was forced to use it to pass the tests.
"def print_snowman_graphic(wrong_guesses_count):
"""This function prints out the appropriate snowman image
depending on the number of wrong guesses the player has made.
"""
for i in range(SNOWMAN_MAX_WRONG_GUESSES - wrong_guesses_count, SNOWMAN_MAX_WRONG_GUESSES):
print(SNOWMAN_GRAPHIC[i])
"
There was a problem hiding this comment.
this was my code before I needed to arrange for this:
def snowman():
random_word_generator = RandomWord()
snowman_word = random_word_generator.word(
word_min_length=SNOWMAN_MIN_WORD_LENGTH,
word_max_length=SNOWMAN_MAX_WORD_LENGTH)
correct_letter_guess_statuses = build_letter_status_dict(snowman_word)
wrong_guesses_list = []
while (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(wrong_guesses_list,
correct_letter_guess_statuses)
if user_input in correct_letter_guess_statuses:
print("\n*** You guessed a letter that's in the word! ***\n" )
correct_letter_guess_statuses[user_input] = True
else:
print(f"\n*** The letter '{user_input}' is not in the word ***\n")
wrong_guesses_list.append(user_input)
print(f"Wrong guesses: {wrong_guesses_list}\n")
print_word_progress_string(snowman_word, correct_letter_guess_statuses)
print("\n")
print_snowman(wrong_guesses_list)
if is_word_guessed(snowman_word, correct_letter_guess_statuses):
print("Congratulations, you win!")
else:
print("Sorry, you lose! The word was {snowman_word}")
def print_snowman(wrong_guesses_list):
for i in range (SNOWMAN_MAX_WRONG_GUESSES - len(wrong_guesses_list), SNOWMAN_MAX_WRONG_GUESSES):
print(SNOWMAN_GRAPHIC[i])
| while (len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES | ||
| and not is_word_guessed(snowman_word, correct_letter_guess_statuses)): |
There was a problem hiding this comment.
👍 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.
There was a problem hiding this comment.
Wrapping a long condition for readability can be tricky. PEP8 typically recommends indenting a wrapped line, but for a loop condition, this would align the wrapped line with the loop block. A common alternative is to over-indent the wrapped line (indent it two levels), so that the single indentation of the block is at a different level. I've often gone the route you did here of leaving a blank between the condition and the block, though this seems to be less common in the community, so I've tried to switch to the overindent method. Regardless of how we deal with the block, we should always indent a wrapped line at least one more level, so that it's clearly not a standalone line of code.
There was a problem hiding this comment.
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| while (len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES | ||
| and not is_word_guessed(snowman_word, correct_letter_guess_statuses)): | ||
|
|
||
| user_input_string = get_letter_from_user(correct_letter_guess_statuses, |
There was a problem hiding this comment.
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.
| user_input_string = get_letter_from_user(correct_letter_guess_statuses, | ||
| wrong_guesses_list) | ||
|
|
||
| if user_input_string in correct_letter_guess_statuses: |
There was a problem hiding this comment.
👍 Nice decision to check whether the letter was in the word by using the status dict rather than the word itself. Checking for the presence of a value in a dict is more efficient than checking in a str, even when both use the in operator. 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!
| else: | ||
| print(f"\nThe letter '{user_input_string}' is not in the word\n") | ||
| wrong_guesses_list.append(user_input_string) | ||
| wrong_guesses_count += 1 |
There was a problem hiding this comment.
If we removed the wrong_guesses_count and looked only at the len(wrong_guesses_list), we would be able to skip updating wrong_guesses_count here.
If you are going to use a helper variable to track the wrong guesses, then we could reduce some of the risk of dat adrift by updating it by reading the list len() rather than incrementing a count. This would help ensure that the data stays in sync. In a project of this size, it's pretty easy to ensure that all the places the list length changes also update the wrong guess count, but in larger projects, it can be more challenging to ensure multiple related variables are always updated together, which is why we might elect to do without the extra named variable.
| wrong_guesses_list.append(user_input_string) | ||
| wrong_guesses_count += 1 | ||
|
|
||
| print(f"Wrong guesses: {wrong_guesses_list}\n") |
There was a problem hiding this comment.
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.
| print(f"Wrong guesses: {wrong_guesses_list}\n") | ||
| print_word_progress_string(snowman_word, correct_letter_guess_statuses) | ||
| print("\n") | ||
| print_snowman_graphic(wrong_guesses_count) |
There was a problem hiding this comment.
This is the only place wrong_guesses_count is being used. Replacing this with len(wrong_guesses_list) would allow us to remove this variable entirely.
| print("\n") | ||
| print_snowman_graphic(wrong_guesses_count) | ||
|
|
||
| if is_word_guessed(snowman_word, correct_letter_guess_statuses): |
There was a problem hiding this comment.
👍 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.
No description provided.