Skip to content

Veema Jhagru - C24 Crow#25

Open
mikellewade wants to merge 1 commit intoAda-C24:mainfrom
v-e-e-m-a:main
Open

Veema Jhagru - C24 Crow#25
mikellewade wants to merge 1 commit intoAda-C24:mainfrom
v-e-e-m-a:main

Conversation

@mikellewade
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown
Author

@mikellewade mikellewade left a comment

Choose a reason for hiding this comment

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

Veema, great work on completing the first Ada project!

Comment thread game.py
correct_letter_guess_statuses = build_letter_status_dict(snowman_word)

while wrong_guesses_count < SNOWMAN_MAX_WRONG_GUESSES:
user_input = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list)
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.

This line of characters exceeds (due to the indentation) the suggested 80 character limit for readability. You'd want to use the PEP 8 Style Guidelines for line continuation to break this up onto multiple lines.

Suggested change
user_input = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list)
user_input = get_letter_from_user(
correct_letter_guess_statuses,
wrong_guesses_list)

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

while wrong_guesses_count < 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.

Nice work with this while loop! We don't actually know how many times it will take a user to guess the word or hit the limit of wrong guesses (could keep make invalid guesses) so this the perfect choice for that situation!

Comment thread game.py
'Sorry, you lose! The word was {snowman_word}' if the player loses
"""
pass
wrong_guesses_count = 0
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.

This variable to track the number of wrong guesses isn't necessary. Any time we need to know the count, we can get the length of the wrong guesses list. While there is a small bit of efficiency gained by not needing to call len(), the danger (not so much here, but in more complex code) is that we could update one of the list or the count in one location, but forget to update the other, meaning they are now out of sync.

Comment thread game.py
while wrong_guesses_count < SNOWMAN_MAX_WRONG_GUESSES:
user_input = get_letter_from_user(correct_letter_guess_statuses, wrong_guesses_list)

if user_input in snowman_word and not correct_letter_guess_statuses[user_input]:
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.

Note that the if user_guess in correct_letter_guess_statuses is shorthand (in and not in) for:

for letter in snowman_word: 
    if user_guess == letter: 
        return True
    return False

You can use this syntax to perform the above logic over a multiple kinds of iterables/sequences in Python. Later in the curriculum we will learn that they don't all have the same performance (Big O).

For the second half of your conditional, it essentially checks if the letter has been guessed correctly already or not. Keep in mind that the helper function get_letter_from_user performs this logic already. It is a redundancy to check it here. You could remove this piece of code.

Suggested change
if user_input in snowman_word and not correct_letter_guess_statuses[user_input]:
if user_input in snowman_word:

Comment thread game.py
Comment on lines +31 to +36
print("You guessed a letter that's in the word!")
correct_letter_guess_statuses[user_input] = True
else:
print(f"The letter {user_input} is not in the word.")
wrong_guesses_list.append(user_input)
wrong_guesses_count += 1
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.

A common rule of thumb is to perform the an operation and then after it is successful alert the user. This is so that there are no false positives sent to the user just in case of some failure in your code after the alert is since. With the print line being placed after it can only execute upon success of the prior lines.

Comment thread game.py
wrong_guesses_count += 1

print_snowman_graphic(wrong_guesses_count)
print(f"Wrong guesses: {wrong_guesses_list}")
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.

For a better user experience, how could we alter this print statement so that the [] aren't wrapped around the guesses of a player?

Comment thread game.py
Comment on lines +40 to +41
print_word_progress_string(snowman_word, correct_letter_guess_statuses)
if is_word_guessed(snowman_word,correct_letter_guess_statuses):
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.

Consider adding a few blank lines within your function to separate logically distinct sections of the code (e.g., setup, processing, return). According to PEP 8, blank lines can improve readability by visually grouping related operations. Right now, everything runs together, which can make it harder to quickly understand the structure and flow of your function.

Suggested change
print_word_progress_string(snowman_word, correct_letter_guess_statuses)
if is_word_guessed(snowman_word,correct_letter_guess_statuses):
print_word_progress_string(snowman_word, correct_letter_guess_statuses)
if is_word_guessed(snowman_word,correct_letter_guess_statuses):

Comment thread game.py
print(f"Wrong guesses: {wrong_guesses_list}")
print_word_progress_string(snowman_word, correct_letter_guess_statuses)
if is_word_guessed(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.

👍🏿

Comment thread game.py
Comment on lines +44 to +45
if is_word_guessed(snowman_word, correct_letter_guess_statuses):
print("Congratulations, you win!")
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.

Nice work! You separated this logic from the while loop making it more your function overall more readable and modular. You could also have this logic inside of your while loop. What would be the disadvantages/advantages of that kind of implementation in terms of readability?

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