Skip to content

Snowman Feedback - Xi Le#9

Open
kelsey-steven-ada wants to merge 1 commit intoAda-C24:mainfrom
xixilele1990:main
Open

Snowman Feedback - Xi Le#9
kelsey-steven-ada wants to merge 1 commit intoAda-C24:mainfrom
xixilele1990:main

Conversation

@kelsey-steven-ada
Copy link
Copy Markdown

@kelsey-steven-ada kelsey-steven-ada commented Sep 2, 2025

This is a "Pull Request" where folks can compare code between branches of a GitHub repo or forked repos, and leave comments about changed code. I will leave comments on this pull request with my feedback so you can see exactly what line of code my comments apply to. If you have questions about any piece of feedback, please respond here on Github or Slack me!

Copy link
Copy Markdown
Author

@kelsey-steven-ada kelsey-steven-ada left a comment

Choose a reason for hiding this comment

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

Congrats on completing your first project Xi!

For the future, I recommend practicing making more frequent, smaller commits. Especially as we have more complex projects and features, we will want those commits to help us by acting as checkpoints we can go back to if we try something out in our code that doesn't work.

Comment thread game.py
pass
letter_status = build_letter_status_dict(snowman_word)
wrong_guesses = []
wrong_count = 0
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.

When we have a list like wrong_guesses, we should not keep a separate variable to track the length of this list. Anytime we need the length of a list, we should use the built in function len.

Anywhere we use wrong_count in the function below, we could replace it with len(wrong_guesses). This lowers the surface area where bugs could be created since we aren't trying to keep two separate variable in sync with each other.

Comment thread game.py
'Sorry, you lose! The word was {snowman_word}' if the player loses
"""
pass
letter_status = 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.

Since this variable tracks the status of many letters, I recommend choosing a plural name like letter_statuses

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

Comment thread game.py
wrong_guesses = []
wrong_count = 0

while wrong_count < 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 use of a while loop since we don't know how many times we'll need to iterate!

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

Comment thread game.py

while wrong_count < SNOWMAN_MAX_WRONG_GUESSES:
print_snowman_graphic(wrong_count)
print_word_progress_string(snowman_word, letter_status)
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.

I like seeing this at the start of the loop so that when we play the game, the user will see a print out of how many letters are in the word before they start guessing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@kelsey-steven-ada in the while loop about line 30 add a condition
if len(wrong_guesses) > 0 :
print("Incorrect guesses so far:", "".join(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.

Great use of join to format the string!

Comment thread game.py
print('Congratulations, you win!')
return

letter = get_letter_from_user(letter_status, 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.

💭 Action Required: The "Game Description" says that one of the requirements is "The game prints out all of the incorrect letters that have been guessed".

Currently, the game does not show the player wrong letter guesses.

@xixilele1990 In a response to my comment here, please let me know how/where you would update your code so that when a user guesses an incorrect letter, your game would print out something like: "Wrong guesses: x, y, z"

Comment thread game.py

letter = get_letter_from_user(letter_status, wrong_guesses)

if letter 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.

When we receive a letter from the user, we want to check to see if it exists within the word or not. Given the way this program is set up, we end up with two different collections that hold all the letters of the word, a string (snowman_word) and a dictionary (letter_status).

While we could check either one for the letter we've received, searching within a dictionary is ever so slightly more efficient than searching through a string (We'll talk more about why in Unit 1)! With that in mind, we could make the following small tweak to increase the efficiency of our solution:

Suggested change
if letter in snowman_word:
if letter in letter_status:

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

Comment thread game.py
wrong_count += 1

print_snowman_graphic(wrong_count)
print_word_progress_string(snowman_word, letter_status)
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.

Throughout the "Practice with Python" topic, the examples show the Snowman game printing "You guessed a letter that's in the word!" after a user inputs a correct guess.

While your game does accurately show a word's progress by filling in the _ with the correctly guessed letter (like f e _ _ _ e for example), It would be nice if your game also provided feedback to the user by printing the string above like our examples do.

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

Comment thread game.py

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

A return statement in Python stops the execution of code. I'm wondering if you can think of another way the logic could be written so that you can end the game without needing to use return (or a break statement) to break the loop?

This isn't a requirement, but if you'd like more practice with writing conditional logic in Python, I'd suggest thinking through and re-writing the logic so you don't need return to stop the game.

If you choose to do so, there's no need for you to submit anything to me for re-review. This would just be for your own practice 😊

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 so much , maybe comparing the length of str of the input with snowman to stop the game

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 would have some other conditions to consider with the length, but there are several options! A couple of many options could be to create a boolean flag that controls the while loop, or create a compound conditional that check the win and lose conditions.

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