-
Notifications
You must be signed in to change notification settings - Fork 89
Snowman PR - May Lee #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -14,13 +14,32 @@ | |||||||||||
|
|
||||||||||||
|
|
||||||||||||
| 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 | ||||||||||||
| correct_letter_guess_statuses = build_letter_status_dict(snowman_word) | ||||||||||||
| wrong_guesses_list = [] | ||||||||||||
| player_won = False | ||||||||||||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In many cases, holding a boolean value in a variable can be a great way to control a loop. One big exception is when that variable holds a boolean value that can easily be found in a more established way. For example, this variable tells us whether or not the player has currently won. The only place where it is used is in the conditional on line 41. We currently have a method ( Another aspect of this involves comments I make later in this feedback that show that the final conditional can be avoided entirely with minimal changes to the rest of the program. If we approached it that way, we wouldn't need this variable at all! |
||||||||||||
|
|
||||||||||||
| while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES: | ||||||||||||
| user_input = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list) | ||||||||||||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 currently see two possible fixes here:
|
||||||||||||
|
|
||||||||||||
| if user_input in correct_letter_guess_statuses: | ||||||||||||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just wanted to highlight a really cool thing that you did here! When we receive a letter from the user, we want to check to see if the letter exists within the word or not. Given the way this program is set up, we actually end up with two different collections that hold all the letters of the word, a string ( While we could check either one for the letter we've received, searching within a dictionary is ever so slightly more efficient than searching through a string (We'll talk more about why in Unit 1), so great choice here! |
||||||||||||
| print("You guessed a letter that's in the word!") | ||||||||||||
| correct_letter_guess_statuses[user_input] = True | ||||||||||||
| else: | ||||||||||||
| print(f"The letter {user_input} is not in the word") | ||||||||||||
| wrong_guesses_list.append(user_input) | ||||||||||||
|
|
||||||||||||
| if is_word_guessed(snowman_word, correct_letter_guess_statuses): | ||||||||||||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With this particular program, we have two potential finish conditions: 1. We find the word, 2. We use up all our guesses. One is a win condition and one is a lose condition. There are a couple different ways we could handle these two conditions:
You have opted for the latter, which I think makes sense for a few reasons:
The one small change I would make is to use a |
||||||||||||
| print_word_progress_string(snowman_word, correct_letter_guess_statuses) | ||||||||||||
| print("Congratulations, you win!") | ||||||||||||
| player_won = True | ||||||||||||
| break | ||||||||||||
|
|
||||||||||||
| print_snowman_graphic(len(wrong_guesses_list)) | ||||||||||||
| print_word_progress_string(snowman_word, correct_letter_guess_statuses) | ||||||||||||
| print(f"Wrong guesses: {wrong_guesses_list}") | ||||||||||||
|
|
||||||||||||
| if not player_won: | ||||||||||||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a bit of a continuation to the previous piece of feedback, if we have a while loop that is governed by a specific conditional, it's a good idea to try avoiding extra conditionals after the while loop, especially if they make a similar check to the check made in the while loop! |
||||||||||||
| print(f"Sorry, you lose! The word was {snowman_word}.") | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| def print_snowman_graphic(wrong_guesses_count): | ||||||||||||
|
|
||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docstrings are often a great way to explain what the following function does and how it operates. It is very common practice to see docstrings at the start of every function. If that is the case, no need to remove them! Go ahead and keep them in, especially if they exist in both finished and unfinished functions. This is a good indicator that the programmer intended for them to stay! In this case, rather than get rid of the docstring, it may be a good idea to update it to reflect the inputs, outputs and way this function works! You can use the other docstrings for guidance!