Skip to content

Snowman Feedback - Allison Lee#42

Open
apradoada wants to merge 2 commits intoAda-C23:mainfrom
Allisonalex:main
Open

Snowman Feedback - Allison Lee#42
apradoada wants to merge 2 commits intoAda-C23:mainfrom
Allisonalex:main

Conversation

@apradoada
Copy link
Copy Markdown

Welcome to your first Pull Request (PR), Allison! In industry, when you want to merge any changes you have made to an existing codebase, a PR is used as a form of code review where a manager or peer can easily see the changes that have been made and review the new code that has been added! We will discuss them further in Unit 1, but they will be the primary method we use to give feedback on projects! Feel free to look through my comments and don't hesitate to reach out if you have any questions!

Copy link
Copy Markdown
Author

@apradoada apradoada 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, Allison!

Feel free to look through the feedback I have left here. Overall, everything looks good and the suggestions I made are purely for adhering to coding conventions you will find in industry! None of them require updates unless you would like to update them.

Two other small general notes:

When it comes to commenting your code, it can be a good idea to include comments for blocks of code to describe what they are doing. We do want to find a certain level of balance however. Right now you have comments before almost every line! While this can be helpful for your own learning, it can be a good idea to take some of them out in the final submission. My general rule of thumb is to use comments for blocks of code rather than every line.

I see there are a couple of commits. This is a pretty basic project, so that makes sense. As we do more robust assignments, you will hear us say "commit early and often" during the program. In general, smaller and more commits can be helpful to see the full history of what you have done and go back and figure out where errors happen! As we do larger projects, feel free to make more commits!

Comment on lines +24 to +25
correct_letter_guess_statuses = build_letter_status_dict(snowman_word)
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.

These variable declarations look good!

user_input = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list)
# Check if the user input is in the word
if user_input in correct_letter_guess_statuses:
print("This letter is in the 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.

Just a small thing, but when an incorrect letter is guessed, it tells you which letter was guessed. It might be a good idea to keep things consistent by doing the same thing if a correct letter is guessed!

Suggested change
print("This letter is in the word!")
print(f"The letter {user_input} is in the word!")

# Get a letter from the user
user_input = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list)
# Check if the user input is in the word
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.

There is nothing inherently wrong with this particular conditional, but I do wonder what led you to check for the letter in correct_letter_guess_statuses rather than just the snowman_word. Technically they both have the same letters but if we follow the logic of the game, we'll want to check against the word itself rather than a dictionary that has been created by the word. This conditional closely mirrors a conditional in the function called on line 32 so comparing it to snowman_word directly avoids that duplication.

Comment on lines +45 to +47
if is_word_guessed(snowman_word, correct_letter_guess_statuses):
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.

Good call to check for the win condition within the loop and return the function early if the win condition is met! This is a very popular and widely used structure!

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