Creating a pull request to leave review comments for Snowman

[yangashley]

No description provided.

[yangashley]

Really nice work on your first project here at Ada!

Please review my comments on this pull request for feedback about your submission.

Let me know if you have any questions!

[yangashley]

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.

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. In the future, I challenge you to write fewer comments and eventually remove the comments before you submit your work. If it’s challenging to understand without comments then we’ve identified areas for growth 😁

[celestialmage]

Understood. I include a lot of comments in my code because it helps me focus on what my code is supposed to do, but I can definitely understand how comments can muddy and bloat code files. From now on, I will remove all comments from the final versions of my repos!

[yangashley]

Remove code that is not used and commented out. Leaving commented out code can lead to confusion when other developers come along and read code. Why is this commented out? Should I comment it back in?

Or worse, if the code gets accidentally commented in then you would have code that shouldn't execute that ends up getting run which would lead to a bug. In this case, it's a minor bug that prints a string, but if the code was something more consequential like deleting a user account then we'd be dealing with a bigger issue.

Suggested change

# print(snowman_word)

[yangashley]

Notice that there's some repetition in your code. len(wrong_guesses_list) appears 2 times.

When we repeat our code, it makes it difficult to maintain because we need to find all the different places the repeated code appears. For example, if I wanted to change the variable name wrong_guesses_list to wrong_guesses, I'd need to look for all the places the variable I want to change appear. You could miss one and introduce a bug. Therefore, we want to keep our code DRY (Don't Repeat Yourself).

I'd prefer a variable called num_wrong_guesses declared on line 29 and then referencing that variable here on line 56 and also line 33.

num_wrong_guesses = len(wrong_guesses_list)
while num_wrong_guesses < SNOWMAN_MAX_WRONG_GUESSES and puzzle_solved is not True:: 
    # blah blah
    print_snowman_graphic(num_wrong_guesses)

[celestialmage]

I have refactored the code to include the num_wrong_guesses variable and use it in the recommended sections!

[yangashley]

Nice work here! There is a small piece of the game logic missing. In the "Game Description" step 7 says "The game prints out all of the incorrect letters that have been guessed".

Please respond to this comment letting me know where you would add this logic that prints out all the incorrectly guessed letters and what code should you write to implement this feature @celestialmage ?

Thanks!

[celestialmage]

Hi Ashley! In the final version of the improved code, I used the join() method to merge all of the current incorrectly guessed letters into a single string, and then printed it on line 30 with a conditional statement that only prints the list if there has been an incorrect guess so far.

[yangashley]

Awesome, using join is what I'd do too!

[yangashley]

It's more Pythonic to write:

Suggested change

	while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES and puzzle_solved is not True:
	while len(wrong_guesses_list) < SNOWMAN_MAX_WRONG_GUESSES and not puzzle_solved:

Since puzzle_solved will evaluate to True or False, we don't need to compare it with is not.

If we wanted to check that puzzle_solved was True, we could just write something like if puzzle_solved:, but here since we want to check that it's False, we can use not to invert the value.

[yangashley]

We could also use the provided helper function print_word_progress_string to print this out