-
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
Amethyst - Elaine Watkins #105
base: main
Are you sure you want to change the base?
Conversation
LETTER_POOL = { | ||
'A': 9, | ||
'B': 2, | ||
'C': 2, | ||
'D': 4, | ||
'E': 12, | ||
'F': 2, | ||
'G': 3, | ||
'H': 2, | ||
'I': 9, | ||
'J': 1, | ||
'K': 1, | ||
'L': 4, | ||
'M': 2, | ||
'N': 6, | ||
'O': 8, | ||
'P': 2, | ||
'Q': 1, | ||
'R': 6, | ||
'S': 4, | ||
'T': 6, | ||
'U': 4, | ||
'V': 2, | ||
'W': 2, | ||
'X': 1, | ||
'Y': 2, | ||
'Z': 1 | ||
} |
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.
Hey Elaine! Great job completing Adagrams! Congrats on your first project at Ada 🎉 I noticed you created this data structure but you aren't making use of it. It's common practice to remove any unused code so we don't take up any unnecessary space in memory. We can discard this 😁👍🏾
user_hand = [] | ||
|
||
for i in range(10): | ||
random_choice = random.choices(letter_pool, weights=weight, k = 1) | ||
|
||
tile = letter_pool.index(random_choice[0]) | ||
weight[tile] -= 1 | ||
if weight[tile] == 0: | ||
weight.pop(tile) | ||
letter_pool.pop(tile) | ||
|
||
user_hand.append(random_choice[0]) | ||
|
||
return user_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.
Amazing job with this solution! You were able to successfully use random.choices
with your letter pool and their weights 👍🏾 I think this is an optimal approach to this function. I noticed you removed the weight from the letter that was grabbed using .index()
! Solid approach 💯
def uses_available_letters(word, letter_bank): | ||
pass | ||
uppercase_word = word.upper() | ||
for char in uppercase_word: | ||
if char not in letter_bank or uppercase_word.count(char) > letter_bank.count(char): | ||
return False | ||
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.
Nice use of .count()
! I also noticed you returned True no matter what happens because False will only be returned in that conditional is met 👍🏾 You could also use word.upper()
in your for loop instead of creating another variable in memory 🥳
score_chart = { | ||
'A': 1, | ||
'E': 1, | ||
'I': 1, | ||
'O': 1, | ||
'U': 1, | ||
'L': 1, | ||
'N': 1, | ||
'R': 1, | ||
'S': 1, | ||
'T': 1, | ||
'D': 2, | ||
'G': 2, | ||
'B': 3, | ||
'C': 3, | ||
'M': 3, | ||
'P': 3, | ||
'F': 4, | ||
'H': 4, | ||
'V': 4, | ||
'W': 4, | ||
'Y': 4, | ||
'K': 5, | ||
'J': 8, | ||
'X': 8, | ||
'Q': 10, | ||
'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.
Excellent choice for this data structure 💯
scores_list = list() | ||
scores_list.append(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.
I noticed you created this list but you're not using it anywhere 😄 we generally don't want to create variables we won't be using, we can discard it
cap_word = word.upper() | ||
word_score = 0 | ||
for char in cap_word: | ||
if score_chart[char]: | ||
word_score += score_chart[char] | ||
else: | ||
continue | ||
# if length of word is between 6 - 10 | ||
if len(cap_word) >= 7 and len(cap_word) <= 10: | ||
word_score += 8 | ||
|
||
scores_list = list() | ||
scores_list.append(word_score) | ||
return 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.
Amazing work with this logic Elaine!
adagrams/game.py
Outdated
def get_highest_word_score(word_list): | ||
pass No newline at end of file | ||
high_score = 0 | ||
winning_word = "" | ||
ties = [] | ||
multiple_tie_words = [] | ||
|
||
for word in word_list: | ||
score = score_word(word) | ||
multiple_tie_words.append(word) | ||
|
||
if score in ties: | ||
if len(ties) >= 1: | ||
winning_word = max(word_list, key=len) | ||
if len(ties) >= 2 or len(word) != 10: | ||
if len(word_list) == 2 and len(winning_word) == 10: | ||
winning_word = max(word_list, key=len) | ||
else: | ||
winning_word = min(word_list, key=len) | ||
|
||
if score > high_score: | ||
high_score = score | ||
winning_word = word | ||
|
||
ties.append(score) | ||
|
||
return winning_word, high_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.
Nice job translating the highest scoring word logic! Awesome job using min
and max
! 😁 your comprehension of flow control is stellar & you seem to have put everything from the learn lessons together quite nicely! With that being said, I’d like to offer this alternative approach: We could create the tuple we have to return at the beginning. You can grab the first word in the word_list and set that as the first value in the tuple. The second value in the tuple would be 0 as a starter score.
You can then reassign that tuple's value to the word with the higher score and continue that process until you capture the highest score. You would finally return that variable 👍🏾 Just something to consider 🤩
For commits, make sure to commit more often and make your commits more descriptive! Instead of taking about what wave was completed, include details about what functionality was added. |
No description provided.