Snowman Feedback - Geethalekshmi Santhakumari#5
Snowman Feedback - Geethalekshmi Santhakumari#5yangashley wants to merge 1 commit intoAda-C24:mainfrom
Conversation
yangashley
left a comment
yangashley
left a comment
There was a problem hiding this comment.
Nice job! Please review my comments, and let me know if you have any questions.
|
|
||
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Unnecessary blank lines should be removed.
Avoid adding unnecessary blank lines to your project because it makes your codebase longer and doesn't comply with the PEP8 style guide.
The guide says "Surround top-level function and class definitions with two blank lines." and "Extra blank lines may be used (sparingly) to separate groups of related functions."
Review the style guide recommendations on blank lines here.
| 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.
| while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES : | |
| while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES: |
Remove 2 unnecessary whitespaces. Small inconsistencies in a codebase add up and they make the code more difficult to read/understand and harder to maintain (update/refactor).
Code that is ready for review, like what is here in this PR, should comply with style conventions as described by the official Python style guide.
You can read more about whitespaces here
|
|
||
| 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.
| if user_input in snowman_word : | |
| if user_input in snowman_word: |
Remove extra whitespace
|
|
||
| if user_input in snowman_word : | ||
| print("You guessed a letter that's in the word!") | ||
| correct_letter_guess_statuses [user_input] =True |
There was a problem hiding this comment.
Missing white space after equal sign.
| correct_letter_guess_statuses [user_input] =True | |
| correct_letter_guess_statuses [user_input] = True |
Take care to look over your own code before you submit it for review, especially when you're at internship. Small inconsistencies in a large codebase are like a death by 1000 paper cuts. Each inconsistency makes the codebase more difficult to maintain.
I can help you catch inconsistencies here, but my primary objective for a code review is to review your logic so it would be good for you to add a linter and/or review your own code so you know it looks professional.
| if user_input in snowman_word : | ||
| print("You guessed a letter that's in the word!") | ||
| correct_letter_guess_statuses [user_input] =True | ||
|
|
There was a problem hiding this comment.
Between your if statement on line 31 and your else here on line 35, you have a blank line.
However the if statement on line 43 and else on line 47 are not separated by a blank line, which is an inconsistency across your code.
Given that we don't need this blank line on line 33 to bring separation between if/else, I'd remove it to be consistent across your entire project and in line with the PEP8 style guide recommendations on blank lines.
| print(f"The letter {user_input} is not in the word") | ||
| wrong_guesses_list.append(user_input) | ||
|
|
||
| print_snowman_graphic(len(wrong_guesses_list)) |
There was a problem hiding this comment.
👍 Nice job using finding the length of wrong_guesses_list and passing that value to print_snowman_graphic. Doing so means you don't need to create an unnecessary variable like wrong_guesses_count that would need to be manually incremented (which would be a place that a bug could get introduced).
| if is_word_guessed(snowman_word, correct_letter_guess_statuses) : | ||
| print("\n") | ||
| print (f"Congratulations, you win!") | ||
| return None |
There was a problem hiding this comment.
A return statement in Python stops the execution of code, which is why I suspect you return None here. 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 return None 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 😊
|
|
||
| print_word_progress_string(snowman_word,correct_letter_guess_statuses) | ||
|
|
||
| if is_word_guessed(snowman_word, correct_letter_guess_statuses) : |
There was a problem hiding this comment.
| if is_word_guessed(snowman_word, correct_letter_guess_statuses) : | |
| if is_word_guessed(snowman_word, correct_letter_guess_statuses): |
Remove unnecessary whitespace before colon.
2 participants
No description provided.