Snowman Feedback - Amanda Thompson#8
Conversation
kelsey-steven-ada
left a comment
kelsey-steven-ada
left a comment
There was a problem hiding this comment.
Congrats on completing your first project Amanda!
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.
| """ | ||
| pass | ||
| correct_letter_status = build_letter_status_dict(snowman_word=snowman_word) | ||
| wrong_guesses_list = [] |
There was a problem hiding this comment.
Different development teams will have different guidelines, but we typically want to describe the contents of a variable and can leave off the data type in the name. This is especially true in languages other than Python that require folks to declare a variable's type when we create it.
It would be completely valid and in some cases preferable to call this variable wrong_guesses rather than wrong_guesses_list.
| 'Sorry, you lose! The word was {snowman_word}' if the player loses | ||
| """ | ||
| pass | ||
| correct_letter_status = build_letter_status_dict(snowman_word=snowman_word) |
There was a problem hiding this comment.
Since this variable tracks the status of many letters, I recommend choosing a plural name like correct_letter_statuses.
| correct_letter_status = build_letter_status_dict(snowman_word=snowman_word) | ||
| wrong_guesses_list = [] | ||
|
|
||
| while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES: |
There was a problem hiding this comment.
Nice use of a while loop since we don't know how many times we'll need to iterate!
| print(f"Incorrect Letters Guessed: {wrong_guesses_list}") | ||
| print_snowman_graphic(len(wrong_guesses_list)) | ||
|
|
||
| print_word_progress_string(snowman_word, correct_letter_status) |
There was a problem hiding this comment.
In the current flow, the user will not know how many letters are in the word they are guessing until after they have made their first guess. How could we make that a better experience for users by showing them that information before they start guessing?
| correct_letter_status[guessed_letter] = True | ||
| else: | ||
| wrong_guesses_list.append(guessed_letter) | ||
| print(f"Incorrect Letters Guessed: {wrong_guesses_list}") |
There was a problem hiding this comment.
Nice feedback for the user so they can keep track and avoid guessing the same letters!
If we display wrong_guesses_list directly, python automatically surrounds the contents with [] and each letter with ''. This might look a little odd to a player, so consider looking into the join function provided on python strings.
| wrong_guesses_list = [] | ||
|
|
||
| while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES: | ||
| guessed_letter = get_letter_from_user(correct_letter_status, wrong_guesses_list) |
There was a problem hiding this comment.
When it comes to writing Python code, one of the guides you'll see us reference is the PEP 8 Style Guide. This is a guide for best practices when it comes to styling our code. One of the most common things we see is to try and keep individual lines of code under 79 characters including indentation. This line currently goes just past that limit. It won't cause the code to break or anything, but it is a good thing to keep in mind in terms of readability and best practices!
I see two possible fixes here:
-
Shorter and more concise variable names:
- While there is no hard and fast rule on how long variable names should be, we could first try shortening the variable names we use here (
correct_letter_guess_statusesandwrong_guesses_list) as they are on the longer side. That being said, these names match the parameters for the function you use to call them, which is good practice, so we may want a different approach in this piece of code.
- While there is no hard and fast rule on how long variable names should be, we could first try shortening the variable names we use here (
-
Restyle the Function Call:
- This is a great trick to have up your sleeve when making a function call that either includes multiple parameters or starts to approach that 79 character line limit! We can drop the parameters to the next line(s) to look something like the following:
Suggested changeguessed_letter = get_letter_from_user(correct_letter_status, wrong_guesses_list) user_input = get_letter_from_user( correct_letter_statuses, wrong_guesses_list ) - This is known as Implicit Line Continuation in Python. Whenever we have information nested inside parentheses, brackets or curly braces, we can drop the nested information to new lines without any specific notation!
|
|
||
| while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES: | ||
| guessed_letter = get_letter_from_user(correct_letter_status, wrong_guesses_list) | ||
| if guessed_letter in correct_letter_status: |
There was a problem hiding this comment.
Great use of the correct_letter_status dict to check if the user input is part of snowman_word a little more efficiently than if we tried to search the string itself!
| if is_word_guessed(snowman_word, correct_letter_status): | ||
| print('Congratulations, you win!') | ||
| break | ||
|
|
||
| if len(wrong_guesses_list) == SNOWMAN_MAX_WRONG_GUESSES: | ||
| print(f"Sorry, you lose! The word was {snowman_word}") |
There was a problem hiding this comment.
If the user has won, then we know that the if-statement outside of the loop if len(wrong_guesses_list) == SNOWMAN_MAX_WRONG_GUESSES will evaluate to False and will not be entered. When we use break to exit the loop early on a win, the loop ends, but the function continues to run and evaluate the remaining if-statement, even though we know it will not be entered.
Another option here would be to use return after the win condition which will immediately leave the function and we avoid taking time to run statements that will take no action.
| while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES: | ||
| guessed_letter = get_letter_from_user(correct_letter_status, wrong_guesses_list) | ||
| if guessed_letter in correct_letter_status: | ||
| correct_letter_status[guessed_letter] = True |
There was a problem hiding this comment.
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.
|
|
||
| if is_word_guessed(snowman_word, correct_letter_status): | ||
| print('Congratulations, you win!') | ||
| break |
There was a problem hiding this comment.
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 break to stop the loop.
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 😊
2 participants
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!