Snowman Feedback - Alice Le#27
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 Alice!
- There is a missing requirement, please check out my feedback and let me know if you have questions
- 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!
| 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 | ||
| """ | ||
| pass |
There was a problem hiding this comment.
I recommend keeping the snowman function here at the top of the file.
Specific teams might have their own style guides, but it is a common practice to place helper functions at the end of a file rather than the beginning. An often-used analogy is to lay out code like a newspaper article.
- Headlines and the most important information comes first - Functions that use other functions in the file go at the top.
- That gets followed by the details and extra information in an article, or in our code, all the helper functions that support the higher up functions.
| # Add a new list to track correctly guessed letters | ||
|
|
||
| wrong_guesses_list = [] | ||
| correct_guess_count = 0 |
There was a problem hiding this comment.
When we have a list like wrong_guesses_list, 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 correct_guess_count in the function below, we could replace it with len(wrong_guesses_list). 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.
|
|
||
| # Add a new list to track correctly guessed letters | ||
|
|
||
| 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.
| # This print statement is only for personal testing and | ||
| # should be removed before sharing the code with others | ||
| print(f"debug info: {snowman_word}") |
There was a problem hiding this comment.
While we are writing code, print statements like this can be a great way to debug! They should not make it into the final code base however, so we should be sure to remove them before submission in future assignments.
| # Add a new list to track correctly guessed letters | ||
|
|
||
| wrong_guesses_list = [] |
There was a problem hiding this comment.
| # Add a new list to track correctly guessed letters | |
| wrong_guesses_list = [] | |
| # Add a new list to track correctly guessed letters | |
| wrong_guesses_list = [] |
We shouldn't leave blank lines between a comment and the code that it belongs to. It's easy for folks to get confused about what the comment is referring to, which often leads to losing time by needing to track folks down to ensure what the comment's original intention was.
| while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES and correct_guess_count < len(correct_letter_guess_statuses): | ||
| user_input = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list) | ||
|
|
||
| if user_input 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!
| print(f"The letter {user_input} is not in the word") | ||
| wrong_guesses_list.append(user_input) | ||
|
|
||
| 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.
Yes, I have updated to call the function print_word_progress_string and it print out how many words that player need to guess in line 115 . print_word_progress_string(snowman_word, correct_letter_guess_statuses)
| correct_letter_guess_statuses = build_letter_status_dict(snowman_word) | ||
|
|
||
| while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES and correct_guess_count < len(correct_letter_guess_statuses): | ||
| user_input = 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.
@TRANGLE891 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.
yes, I already fix it. I add 1 more print(f"Wrong guesses + {wrong_guesses_list}") in line 129
| while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES and correct_guess_count < len(correct_letter_guess_statuses): | ||
| user_input = 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. These lines currently go 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!
One option to reformat the code to keep our lines shorter could look like wrapping the expressions in parentheses and using line breaks between the expressions:
while (len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES
and correct_guess_count < len(correct_letter_guess_statuses)):
user_input = get_letter_from_user(
correct_letter_guess_statuses,
wrong_guesses_list
)| print("You guessed a letter that's in the word!") | ||
| else: | ||
| print(f"The letter {user_input} is not 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 =]
… players must guess
|
|
||
| print_word_progress_string(snowman_word, correct_letter_guess_statuses) | ||
| print(f"Wrong guessed: {wrong_guesses_list}") |
There was a problem hiding this comment.
Thanks for addressing this, the project is looking good! =]
Around printing a list directly, if we print wrong_guesses_list, 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.
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!