-
Notifications
You must be signed in to change notification settings - Fork 69
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
Otters - Amel N - Mitra M #44
base: master
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.
Great job, Amel & Mitra! 🎉
Your tests pass, your commit messages are descriptive and frequent, and your code is crystal clear! 🔮 I'm impressed with the subtle styling in this submission, and it's clear to me that y'all are comfortable with working with lists, dictionaries, and tuples! This project is a Green. 🟢
Keep up the great work!
|
||
# Draw a random letter | ||
while len(letters) < 10: | ||
letter = random.choice(list(LETTER_POOL)) |
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.
Clever use of drawing out all the keys of LETTER_POOL
as a list, here!
|
||
def uses_available_letters(word, letter_bank): | ||
pass | ||
""" check each letter used in leter_bank_copy """ | ||
letter_bank_copy = letter_bank[:] |
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.
Great use of the python slicing notation to make a copy! ✨
for word in word_list: | ||
score = score_word(word) | ||
score_tuple = (word, score) | ||
word_and_score.append(score_tuple) | ||
|
||
# Create dictionary using word-score pair in each tuple as key and value | ||
word_and_score_dict = {} | ||
for word, score in word_and_score: | ||
word_and_score_dict[word] = 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.
Since we're creating a word_and_score
list of tuples only to form the word_and_score_dict
on L108-110, I'm curious if we could have made this more efficient and tried to make word_and_score_dict
from the get-go! 🤔
highest_score = score | ||
best_word = word | ||
# Break tie |
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.
Great tie breaker logic below! I see how you could have combined L121-122 and L127-128 if you wanted, but this reads very clearly!
while len(letters) < 10: | ||
letter = random.choice(list(LETTER_POOL)) | ||
if LETTER_POOL[letter] == 0: # Check letter is in pool | ||
continue | ||
else: # Remove one letter from the letter pool | ||
letters.append(letter) | ||
LETTER_POOL[letter] -= 1 | ||
|
||
return letters |
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.
One thing to consider: the way this function is structured, this will prevent us from picking a letter that is not available. However, when we randomly pick on L65, it's a random pick from the 26 letters in an alphabet. That means A
and Z
both have 1/26th chance of being picked before we do the validation check, despite the letter bag distribution for A
being 9/98 and Z
being 1/98.
No description provided.