-
Notifications
You must be signed in to change notification settings - Fork 89
Snowman Feedback - Iris Cheung #14
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 | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,14 +20,37 @@ def snowman(snowman_word): | |||||||||||
| 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 = [] | ||||||||||||
|
|
||||||||||||
| while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES and not is_word_guessed(snowman_word, correct_letter_guess_statuses): | ||||||||||||
| user_input = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list).lower() | ||||||||||||
|
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: | ||||||||||||
|
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) | ||||||||||||
|
|
||||||||||||
| print_snowman_graphic(len(wrong_guesses_list)) | ||||||||||||
| print_word_progress_string(snowman_word, correct_letter_guess_statuses) | ||||||||||||
|
Comment on lines
+38
to
+39
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. Great choice to print both of these outside the conditionals! |
||||||||||||
|
|
||||||||||||
| if is_word_guessed(snowman_word, correct_letter_guess_statuses): | ||||||||||||
|
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. If we moved this conditional from where it currently is to inside the loop, we could handle one of the ending conditions inside the loop and remove it from the while conditionals. We could then exit the function early with an explicit return statement! That allows us only deal with the lose condition outside of the loop. In fact, we would know that the user has lost if the loop ends, so we could just include the print statement on line 43 outside the loop! |
||||||||||||
| print("Congratulations, you win!") | ||||||||||||
| else: | ||||||||||||
| print(f"Sorry, you lose! The word was {snowman_word}") | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
|
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. This is a small thing, but make sure to remove the pass statements! It won't cause the code to fail or anything, but it is always best to remove unused code! |
||||||||||||
| pass | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| def print_snowman_graphic(wrong_guesses_count): | ||||||||||||
| """This function prints out the appropriate snowman image | ||||||||||||
| depending on the number of wrong guesses the player has made. | ||||||||||||
| """ | ||||||||||||
|
|
||||||||||||
| for i in range(SNOWMAN_MAX_WRONG_GUESSES - wrong_guesses_count, SNOWMAN_MAX_WRONG_GUESSES): | ||||||||||||
| print(SNOWMAN_GRAPHIC[i]) | ||||||||||||
|
|
||||||||||||
|
|
@@ -103,7 +126,7 @@ def generate_word_progress_string(snowman_word, correct_letter_guess_statuses): | |||||||||||
| output_string += "_" | ||||||||||||
|
|
||||||||||||
| is_not_first_letter = True | ||||||||||||
|
|
||||||||||||
| return output_string | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
|
|
||||||||||||
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.
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 former, which Is good, but it does increase a bit of the work that needs to be done outside the loop. This happens for the following reasons:
If we check both for each iteration, our loop ends if either the win or lose condition is met. This means that we are going to have to make an additional and unnecessary check after the loop to see which condition was met. If we somehow resolved one win condition inside the loop, we would remove the need for extra checks after the while loop has complete.
If we meet one of the conditions inside the loop, we can exit the function early with an explicit return. This resolves one of our conditions that we no longer have to worry about later on.
Overall your approach works, but it does add a little bit of extra code!