Conversation
apradoada
left a comment
apradoada
left a comment
There was a problem hiding this comment.
Great job, Ellie!
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! Before I can give this code a passing score, I just need you to fix the break statement so that it passes all the tests and then you're good to go!
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. You currently don't have many, so in the future, just make sure to include a few!
I see there is just one commit. 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 breaks happen! As we do larger projects, feel free to make more commits!
| wrong_guesses_list = [] | ||
| correct_letter_guess_statuses = build_letter_status_dict(snowman_word) | ||
|
|
||
| while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES and not is_word_guessed(snowman_word, correct_letter_guess_statuses): |
There was a problem hiding this comment.
This compound conditional looks good! One small thing is that this line breaks the PEP8 guidelines for line length (79 characters or less including indentation https://peps.python.org/pep-0008/#maximum-line-length). From the linked guide:
The preferred way of wrapping long lines is by using Python’s implied line continuation inside parentheses, brackets and braces. Long lines can be broken over multiple lines by wrapping expressions in parentheses.
It can definitely be tough to break up lines nicely, but when you have compound conditionals, I think the most obvious way to break things up is by the conditional itself. For example, you could do:
| while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES and not is_word_guessed(snowman_word, correct_letter_guess_statuses): | |
| while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES and \ | |
| not is_word_guessed(snowman_word, correct_letter_guess_statuses): |
This makes it just a little easier to read!
| for letter in snowman_word: | ||
| if letter == user_input: | ||
| correct_letter_guess_statuses[letter] = True |
There was a problem hiding this comment.
I fully get where you are coming from with this particular approach! However, remember that the function on line 26 creates a dictionary called correct_letter_guess_statuses. With a dictionary, we don't need to loop through the snowman word to find the letter we are looking for. We can get away with just accessing the letter we've input directly and changing its status to True!
| for letter in snowman_word: | |
| if letter == user_input: | |
| correct_letter_guess_statuses[letter] = True | |
| correct_letter_guess_statuses[user_input] = True |
| else: | ||
| wrong_guesses_list.append(user_input) | ||
|
|
||
| print_snowman_graphic(len(wrong_guesses_list)) # Show Snowman progress |
There was a problem hiding this comment.
Right now, because this line is nested inside our else statement, it will only print out the snowman graphic if the user guesses a letter that is not in the word. Just for the sake of the game, it might be a good idea to move this line outside of the conditionals but still inside the while loop!
| print_word_progress_string(snowman_word, correct_letter_guess_statuses) # Show word progress | ||
| user_input = get_letter_from_user(correct_letter_guess_statuses,wrong_guesses_list) | ||
|
|
||
| if user_input in snowman_word: |
There was a problem hiding this comment.
This is just a small thing, but for the sake of the game, it might be a nice touch to add a little message to the user to let them know if they got the letter right or wrong!
|
|
||
| print_snowman_graphic(len(wrong_guesses_list)) # Show Snowman progress | ||
|
|
||
| if is_word_guessed(snowman_word, correct_letter_guess_statuses): |
There was a problem hiding this comment.
I love the way you've included checking for the win condition here within the loop! As we talked about, the one way you can get this to pass all the tests will be to change the break statement! One other thought here though is that because you are checking to see if the word wins here, you actually don't need to include the win condition within the while loop conditionals itself. This currently leads to a duplicate check when only one is needed!
2 participants
Welcome to your first Pull Request (PR), Ellie! 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!