Snowman Feedback - Esmeralda Arreguin-Martinez

[yangashley]

No description provided.

[yangashley]

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

[yangashley]

Lines 61-64 should be deleted since they're unnecessary blank lines.

[yangashley]

Lines 53-54 should be deleted because they're unnecessary.

While it is minor to have some extra blank lines here or there and it doesn't affect the logic of the code, it does quickly bloat the codebase and make the project visually look longer. This makes the code style inconsistent and more time consuming to maintain. Maintaining a project means working on an existing project either by adding more code or updating existing code. When a project takes a lot of effort to maintain due to style issues and inconsistencies, a dev team is less efficient at their work.

We should adhere to style guides so that our code matches the prescribed style which makes the project easier to maintain.

[yangashley]

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

[yangashley]

Suggested change

#print(f"You made {correct_guesses} correct and {wrong_guesses} incorrect guesses")

A note about comments: Folks might find it useful to leave comments that explain what different lines/sections of code do in the beginning of their coding career. However, when we write self documenting code (code with descriptively named variables and function names) that is concise and easy to follow, then there should be no need for comments. For example, if a function is named get_letter_from_user, I can deduce what the function should do without needing to read a comment that explains what it does.

Additionally, any debugging comments should be removed because they can introduce confusion to other devs reading your code. I might ask myself, should we leave line 44 in? If line 44 were to accidentally get uncommented then this code would execute when maybe it shouldn't have. Here, we have a fairly innocuous line of code, but imagine line 44 was a time consuming call to a database to retrieve some data. If it was uncommented and ran, then we'd be introducing latency into our program.

Finally, comments add more lines of code in a codebase. More lines of code make a codebase more challenging to maintain and update and also create more areas where a bug can accidentally get introduced.

Teams have differing opinions about comments. Some teams may be ok with some comments here and there and some teams prefer no comments unless absolutely necessary. You’ll find out how your team feels when you get there!

My opinion, when it comes to reviewing your work, I prefer to not have comments because if the code is concise and clear then I should be able to understand it.

[yangashley]

Your debugging print statement is not commented out so anyone playing the game can see the secret word 🤪 Remove debugging statements and comments when you're done with them so that your project is readable and maintainable.

At internship and here as well, code that is ready for review in a pull request (like this one) should be free of unnecessary comments and debugging code.

Suggested change

print(f"Debug info: {snowman_word}, length of word: {len(snowman_word)}")

[yangashley]

Nice job using a flag (a variable that is used to indicate that a condition has been met) to control your while loop 👍

Suggested change

	arriving_at_complete_word_guess = False
	is_game_finished = False

It's common practice to name a variable that references a boolean with is so that other devs know that this is flag variable that can change.

[yangashley]

While line 32 is still under the 120 max line length and isn't too difficult to understand, you might consider introducing another variable to make this condition even more clear.

is_player_still_guessing = len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES
while is_player_still_guessing and not arriving_at_complete_word_guess:

[yangashley]

Unnecessary blank lines should be removed on lines 38-39.

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.

Suggested change