Skip to content

Tatyana Ageyeva C23 Snowman Feedback#38

Open
mikellewade wants to merge 4 commits intoAda-C23:mainfrom
TatyanaA90:main
Open

Tatyana Ageyeva C23 Snowman Feedback#38
mikellewade wants to merge 4 commits intoAda-C23:mainfrom
TatyanaA90: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, Tatyana! If you have any questions about the comments I left please let me know!

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:
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!

Comment thread game.py Outdated

user_input = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list)

if user_input in 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.

Good checking to see if the user_input is a key inside of the dictionary you built with build_letter_status_dict. Though we haven't talked about it yet, there are actually advantages of searching for an element is present in a dictionary vs a list/string. If you are curious, check out the term "Time Complexity" in reference to dictionaries and lists/strings.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for the feedback here, I've changed the search in correct_letter_guess_statuses to use the get method to take advantage of the fast search in dict.

Comment thread game.py Outdated

if is_word_guessed(snowman_word, correct_letter_guess_statuses):
print("Congratulations, you win!")
break
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.

We could also use the return keyword here to not only exit out of the loop but out of the entire function itself.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hm, good idea thanks, I've made a change

Comment thread game.py Outdated
Comment on lines +48 to +49
if len(wrong_guesses_list) == SNOWMAN_MAX_WRONG_GUESSES:
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.

If you make the modification of replacing break with return you could then remove the if statement and just have the print statement since if the return didn't execute then that means the user passed the the allotted number of guesses.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

yeah, did that too. Thanks!

Comment thread game.py Outdated
print(f"The letter {user_input} is not in the word")
wrong_guesses_list.append(user_input)

print(f"Wrong guesses: {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.

For a better user experience, how could we alter this print statement so that the [] aren't wrapped around the guesses of a player?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks, I've changed output format and used .join, please check .

@TatyanaA90
Copy link
Copy Markdown

Thank you @mikellewade for constractive and informative feedback! I've got a new knowledge. It was interesting to look out for the all posiible ways to improve and simplify 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