Skip to content

Snowman Feedback - Camille Dubois#30

Open
apradoada wants to merge 2 commits intoAda-C23:mainfrom
c-dubois:main
Open

Snowman Feedback - Camille Dubois#30
apradoada wants to merge 2 commits intoAda-C23:mainfrom
c-dubois:main

Conversation

@apradoada
Copy link
Copy Markdown

Welcome to your first Pull Request (PR), Camille! In industry, when you want to merge any changes you have made to an existing codebase, a PR is used as a form of code review where a manager or peer can easily see the changes that have been made and review the new code that has been added! We will discuss them further in Unit 1, but they will be the primary method we use to give feedback on projects! Feel free to look through my comments and don't hesitate to reach out if you have any questions!

Copy link
Copy Markdown
Author

@apradoada apradoada left a comment

Choose a reason for hiding this comment

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

Great job, Camille!

Feel free to look through the feedback I have left here. Overall, everything looks good and the suggestions I made are purely for adhering to coding conventions you will find in industry! None of them require updates unless you would like to update them.

Two other small general notes:

  1. When it comes to commenting your code, it can be a good idea to include comments for blocks of code to describe what they are doing. You currently don't have any, so in the future, just make sure to include a few!

  2. I see there are a couple of commits. This is a pretty basic project, so that makes sense. As we do more robust assignments, you will hear us say "commit early and often" during the program. In general, smaller and more commits can be helpful to see the full history of what you have done and go back and figure out where errors happen! As we do larger projects, feel free to make more commits!

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
Author

Choose a reason for hiding this comment

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

I think the way you have handled exit conditions here is good! We technically have a win and a lose condition and could include a compound conditional that handles both, but I see that you handled the win condition within the loop itself which is great!

There may be situations where handling both exit conditions at the same time in necessary, but you've correctly identified that this is not one of those situations! Well done!

Comment thread game.py
if is_word_guessed(snowman_word, correct_letter_guess_statuses):
print(f"Congratulations, you win!")
print_word_progress_string(snowman_word, correct_letter_guess_statuses)
break
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I like the instinct to use the break statement here! One small change you could make here is to use the return statement instead. It's a subtle difference and really doesn't effect things like the efficiency of your function, but in terms of the flow of your code, break simply pops you out of the loop itself and continues on with the rest of the function. Return stops the function right then and there. Given the fact that the code after the loop only deals with a lose condition, stopping the function here at the win condition would be slightly preferred!

As we continue programming, you'll find that there are many situations where returning within a loop is a great way to exit a function early! One concrete example would be if you are searching for a specific value in a collection. As soon as you find the value, the function has done it's job and you can exit it!

Comment thread game.py
print_snowman_graphic(len(wrong_guesses_list))
print(f"Wrong guesses: {wrong_guesses_list}")

if len(wrong_guesses_list) == SNOWMAN_MAX_WRONG_GUESSES:
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The instinct here is great, but technically this conditional is very similar to the condition that ends the while loop above! And this is another reason the return statement would be more powerful than break! You would have handled the win condition within the loop and stopped the function there! This means we know that any code after the while loop indicates a loss in the game, so the extra conditional actually isn't necessary! All you would need is to print out the lose string.

In the example I gave above, the equivalent would be returning either False or -1 if the value we are looking for isn't found. We don't need an extra conditional because if the program has made it all the way through the loop without returning, it knows it didn't find what it is looking for!

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