Skip to content

Snowman Feedback - Divya Potnuru#12

Open
yangashley wants to merge 3 commits intoAda-C24:mainfrom
divya42536:main
Open

Snowman Feedback - Divya Potnuru#12
yangashley wants to merge 3 commits intoAda-C24:mainfrom
divya42536:main

Conversation

@yangashley
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown
Collaborator Author

@yangashley yangashley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job! Please review my comments, and let me know if you have any questions.

Comment thread game.py
'Sorry, you lose! The word was {snowman_word}' if the player loses
"""
pass
correct_letter_guess_statuses = build_letter_status_dict(snowman_word)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
correct_letter_guess_statuses = build_letter_status_dict(snowman_word)
correct_letter_guess_statuses = build_letter_status_dict(snowman_word)

Remove extra whitespace. 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.

Comment thread game.py
correct_letter_guess_statuses = build_letter_status_dict(snowman_word)
wrong_guesses_list = []

while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES :
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES :
while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES:

Remove 2 extra whitespaces to keep your code neat and professional.

Comment thread game.py
if user_input in snowman_word :
print("You guessed a letter that's in the word!")
correct_letter_guess_statuses [user_input] =True

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

Unnecessary blank line 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.

Comment thread game.py
print(f"The letter {user_input} is not in the word")
wrong_guesses_list.append(user_input)

print_snowman_graphic(len(wrong_guesses_list))
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 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).

Comment thread game.py
if is_word_guessed(snowman_word, correct_letter_guess_statuses) :
print("\n")
print (f"Congratulations, you win!")
return None
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 😊

Comment thread game.py
"""
pass
correct_letter_guess_statuses = build_letter_status_dict(snowman_word)
wrong_guesses_list = []
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread game.py

if user_input in snowman_word :
print("You guessed a letter that's in the word!")
correct_letter_guess_statuses [user_input] =True
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
correct_letter_guess_statuses [user_input] =True
correct_letter_guess_statuses[user_input] = True

There is an extra white space before [. When you set a key/value pair in a dictionary it should look like dict[key] = value` with no extra or missing whitespaces.

There is a missing whitespace after =.

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.

Comment thread game.py
print("\n")
print (f"Congratulations, you win!")
return None
else:
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you do not use a blank line before the else block, which aligns with style guidelines (unlike the blank line on line 33).

# correct style for writing if/else without extra blank lines
if some_condition:
    # do something
else:
    # do something else

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants