Skip to content

C24 Crow - Mengqiao Han#10

Open
mikellewade wants to merge 1 commit intoAdaGold:mainfrom
Bipentium25:main
Open

C24 Crow - Mengqiao Han#10
mikellewade wants to merge 1 commit intoAdaGold:mainfrom
Bipentium25:main

Conversation

@mikellewade
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown
Author

@mikellewade mikellewade left a comment

Choose a reason for hiding this comment

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

Great job, Mengqiao! You can find the feedback I left here, let me know if you have any questions or concerns about the comments I left.

Comment on lines +26 to +27
victory = False
while wrong_guesses_count < SNOWMAN_MAX_WRONG_GUESSES and not victory:
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 work with this while loop! We don't actually know how many times it will take a user to guess the word or hit the limit of wrong guesses (could keep make invalid guesses) so this the perfect choice for that situation! I also love the victory flag.

Consider adding a few blank lines within your function to separate logically distinct sections of the code (e.g., setup, processing, return). According to PEP 8, blank lines can improve readability by visually grouping related operations. Right now, everything runs together, which can make it harder to quickly understand the structure and flow of your function.

Suggested change
victory = False
while wrong_guesses_count < SNOWMAN_MAX_WRONG_GUESSES and not victory:
victory = False
while wrong_guesses_count < SNOWMAN_MAX_WRONG_GUESSES and not victory:

Comment on lines +23 to +25
wrong_guesses_list = []
wrong_guesses_count = len(wrong_guesses_list)
correct_letter_guess_statuses = build_letter_status_dict(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.

Nice work on these naming conventions, they readily let other developers know what data they are referencing. I also like that you leveraged the len function with wrong_guesses_count to get the number of wrong guesses a user has submitted. Though note that it is not necessary to have this variable, we could just check the len of wrong_guesses_list whenever we need it.

Comment on lines +28 to +29
user_guess = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list)
if user_guess not 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.

Suggested change
user_guess = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list)
if user_guess not in snowman_word:
user_guess = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list)
if user_guess not in snowman_word:

Comment on lines +28 to +29
user_guess = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list)
if user_guess not 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.

Note that the if user_guess not in snowman_word is shorthand (in and not in) for:

for letter in snowman_word: 
    if user_guess == letter: 
        return False
    return True

You can use this syntax to perform the above logic over a multiple kinds of iterables/sequences in Python. Later in the curriculum we will learn that they don't all have the same performance (Big O).

Comment on lines +30 to +32
wrong_guesses_list.append(user_guess)
wrong_guesses_count = len(wrong_guesses_list)
print_snowman_graphic(wrong_guesses_count)
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.

Something that we could add here to enhance the user experience is adding a list of the wrong guesses each wrong guess. This would help the user not make multiple wrong guesses of the same letter.

Comment on lines +35 to +36
correct_letter_guess_statuses[user_guess] = True
print_word_progress_string(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.

else:
correct_letter_guess_statuses[user_guess] = True
print_word_progress_string(snowman_word, correct_letter_guess_statuses)
victory = 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.

victory is really just a flag containing either True or False. You use this flag to determine if the while loop should terminate or not next iteration. We could alternatively do the following:

if is_word_guessed: 
    break

This would remove the and not victory portion of your while loop. However, you need to consider readability of your code as well. Which one more readily communicates to developers that this while loop is break condition is partially dependent on if the word is guessed? @Bipentium25

@Bipentium25
Copy link
Copy Markdown

Thank you for you feedback! I will think of the improvements I can make to my code.

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