Snowman Feedback - Xin Li#10
Conversation
kelsey-steven-ada
left a comment
There was a problem hiding this comment.
Congrats on completing your first project Xin!
| correct_letter_guess_statuses = build_letter_status_dict(snowman_word) | ||
| wrong_guesses_list = [] | ||
|
|
||
| while len(wrong_guesses_list)<SNOWMAN_MAX_WRONG_GUESSES : |
There was a problem hiding this comment.
To make it as easy and quick as possible to understand a line of code, we should use leave a space on either side of logical, assignment, and arithmetic operators. Leaving spaces helps aid our visual processing; by dividing up the parts of our expressions, it's easier to see the parts that make it up and can make our code quicker to read and understand.
| wrong_guesses_list = [] | ||
|
|
||
| while len(wrong_guesses_list)<SNOWMAN_MAX_WRONG_GUESSES : | ||
| user_input_string = get_letter_from_user(correct_letter_guess_statuses, 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. This line currently goes 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 changeuser_input_string = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list) user_input = get_letter_from_user( correct_letter_guess_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!
| pass | ||
|
|
||
| correct_letter_guess_statuses = build_letter_status_dict(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.
| correct_letter_guess_statuses = build_letter_status_dict(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_snowman_graphic(len(wrong_guesses_list)) | ||
| print_word_progress_string(snowman_word, correct_letter_guess_statuses) |
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?
There was a problem hiding this comment.
I add
print_word_progress_string(snowman_word, correct_letter_guess_statuses)before the while loop inside the snowman function. The whole block of code looks like this:
def snowman(snowman_word):
"""Complete the snowman function
replace "pass" below with your own code
It should print 'Congratulations, you win!'
If the player wins and,
'Sorry, you lose! The word was {snowman_word}' if the player loses
"""
correct_letter_guess_statuses = build_letter_status_dict(snowman_word)
wrong_guesses_list = []
print_word_progress_string(snowman_word, correct_letter_guess_statuses)
while len(wrong_guesses_list)<SNOWMAN_MAX_WRONG_GUESSES :
user_input_string = get_letter_from_user (
correct_letter_guess_statuses, wrong_guesses_list)
if user_input_string in correct_letter_guess_statuses:
correct_letter_guess_statuses[user_input_string] = True
print(f"The letter {user_input_string} is in the word")
else:
wrong_guesses_list.append(user_input_string)
print(f"The letter {user_input_string} you guessed is not in the word")
print("Wrong guesses:", ", ".join(wrong_guesses_list))
print_snowman_graphic(len(wrong_guesses_list))
print_word_progress_string(snowman_word, correct_letter_guess_statuses)
if is_word_guessed(snowman_word, correct_letter_guess_statuses):
print("Congratulations, you win!")
return
print(f"Sorry, you lose! The word was {snowman_word}") | while len(wrong_guesses_list)<SNOWMAN_MAX_WRONG_GUESSES : | ||
| user_input_string = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list) |
There was a problem hiding this comment.
💭 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.
@mulanwanderingearth 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"
There was a problem hiding this comment.
On line 35 inside this while loop, after the player guesses a wrong answer, I added a print statement to print the wrong_guesses_list.
print("Wrong guesses:", ", ".join(wrong_guesses_list))The whole loop will look like this:
while len(wrong_guesses_list)<SNOWMAN_MAX_WRONG_GUESSES :
user_input_string = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list)
if user_input_string in correct_letter_guess_statuses:
correct_letter_guess_statuses[user_input_string] = True
print(f"The letter {user_input_string} is in the word")
else:
wrong_guesses_list.append(user_input_string)
print(f"The letter {user_input_string} you guessed is not in the word")
print("Wrong guesses:", ", ".join(wrong_guesses_list))
print_snowman_graphic(len(wrong_guesses_list))
print_word_progress_string(snowman_word, correct_letter_guess_statuses)
if is_word_guessed(snowman_word, correct_letter_guess_statuses):
print("Congratulations, you win!")
return
print(f"Sorry, you lose! The word was {snowman_word}") There was a problem hiding this comment.
Nice, great use of join to format the printed guesses!
|
|
||
| while len(wrong_guesses_list)<SNOWMAN_MAX_WRONG_GUESSES : | ||
| user_input_string = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list) | ||
| if user_input_string in correct_letter_guess_statuses: |
There was a problem hiding this comment.
Great use of the correct_letter_guess_statuses 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!
| user_input_string = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list) | ||
| if user_input_string in correct_letter_guess_statuses: | ||
| correct_letter_guess_statuses[user_input_string] = True | ||
| print(f"The letter {user_input_string} is in the word") |
There was a problem hiding this comment.
I appreciate these print statements to make it very clear to the user whether their guess was successful or not.
| print_word_progress_string(snowman_word, correct_letter_guess_statuses) | ||
| if is_word_guessed(snowman_word, correct_letter_guess_statuses): | ||
| print("Congratulations, you win!") | ||
| return |
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 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 😊
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!