Skip to content

Solhee Jin C23 Snowman Feedback#36

Open
mikellewade wants to merge 1 commit intoAda-C23:mainfrom
ellenjin:main
Open

Solhee Jin C23 Snowman Feedback#36
mikellewade wants to merge 1 commit intoAda-C23:mainfrom
ellenjin: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.

Nice work, Solhee! Please let me know if you have any questions about the comments I left.

Comment thread game.py
correct_letter_guess_statuses = build_letter_status_dict(snowman_word)
wrong_guesses_list = []

# while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES and len(correct_letter_guess_statuses) < len(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.

Comments like these usually you want to delete. We usually want to keep comments that we that we feel give some vital insight to our code. This seems like it was maybe an older rendition of your code so I would be okay with deleting this comment.

Comment thread game.py
wrong_guesses_list = []

# while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES and len(correct_letter_guess_statuses) < len(snowman_word):
while not is_word_guessed(snowman_word, correct_letter_guess_statuses) and len(wrong_guesses_list) < len(SNOWMAN_GRAPHIC):
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!

We could also rework your loop to depend on just the wrong number of guesses or if the word was guessed. We could treat the other condition like a flag and exit out of our function early if said condition is met. I would also say that it might be more intuitive to use something like SNOWMAN_MAX_WRONG_GUESSES in place of len(SNOWMAN_GRAPHIC) These changes would look a little something like this:

while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES: 
    ...

    if is_word_guessed: 
        print(f"Congratulations, you win!")
        return

With the return keyword not only do we exit out of the loop but also the function.

Comment thread game.py
# go until user has guessed all the letters, or the snowman drawing is complete
# guess = get_letter_from_user(wrong_guesses_list, correct_letter_guess_statuses)
guess = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list)
if guess 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.

Another way that this could be done is by checking the dictionary that was constructed with build_letter_status_dict. There are some advantages here that we've not talked about yet but it doesn't hurt to think through how it could be done!

If you are interested the advantage its called Time Complexity!

Comment thread game.py
Comment on lines +41 to +44
if is_word_guessed(snowman_word, correct_letter_guess_statuses):
print("Congratulations, you win!")
else:
print(f"Sorry, you lose! The word was {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.

You make this check twice, is_word_guessed in your code. One is in your while loop and the other is here. If you make the modification I suggested in comment about your while loop we could get rid of this check and move your else block logic an indentation level higher.

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