-
Notifications
You must be signed in to change notification settings - Fork 135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New Pull request for final submission! #130
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GREEN! This looks really great, Valerie! I left a few comments but overall it's awesome!
if copy_letter_pool[str_random_letter] > 0: | ||
hand.append(str_random_letter) | ||
copy_letter_pool[str_random_letter] += -1 | ||
return hand |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love the way you used the random.choices function here to account for the different weights of each letter! You were one of just a few people to catch onto this! Overall this works really well and you've accounted for several different possibilities!
adagrams/game.py
Outdated
validate_enough_available = {} | ||
for letter in letter_bank: | ||
available_count = letter_bank.count(letter) | ||
quantity_available[letter] = available_count |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you never use available_count again, you could merge this line with the one above to save a little space.
adagrams/game.py
Outdated
return False | ||
else: | ||
return True | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It took me a second, but this is a very unique way to write this function! If you wanted to cut this down just a bit, you could make a copy of your letter_bank. From there, you can loop through your word and check to see if each letter is in the copy of the letter_bank. If it is, you can remove it from the letter bank and if it isn't you can immediately return False and terminate the loop and function early! Any chance to terminate a function early is a great idea!
"X": 8, | ||
"Y": 4, | ||
"Z": 10, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This dictionary could be moved to outside the functions as a global constant just in case we wanted to expand this project and use the score_chart elsewhere!
score += score_chart[letter] | ||
if len(word) >= 7: | ||
score += 8 | ||
return score |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function looks great!
elif len(word) < len(winner[0]) and len(winner[0]) != 10: | ||
winner = (word, score) | ||
return winner |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like what you did here, separating each part out into their separate pieces! One challenge I have for you would be to see if you can do the comparisons in place. This would require just one for loop and a few conditionals to compare the different words and their scores! There are a few examples on code reviews for reference!
No description provided.