-
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
Kat Munson Amethyst Adagrams #121
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.
Awesome job Kat ✨
Your submission covers all the learning goals and passes all the tests! You have earned a 🟢 grade!
In your PR, you'll find comments on ways to refactor and DRY up your code. Great work on making small and frequent commits with detailed commit messages. Detailed commit messages are super helpful for future bug hunting!
Keep up the great work Kat! 💜✨
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.
Nice work! Notice how this dictionary makes the function quite long and clunky? We can move this dictionary outside of the function so that the focus is moreso of the logic of the function rather than having to scroll through long data structure.
Also, we're not changing the data in this dictionary so we can consider it a ✨ constant variable ✨ and can change its name to all uppercase instead.
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.
ooooohhhhh I see I see love itttt
frequency_list = [] | ||
hand = [] | ||
# adds letter to frequency_list based on the desired frequency | ||
for letter, frequency in letter_pool.items(): | ||
frequency_list += letter * frequency |
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 work accounting for the statistical probability for each letter!
|
||
def uses_available_letters(word, letter_bank): | ||
pass | ||
letter_bank_copy = copy.deepcopy(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.
Nice work creating a copy of letter_bank
to avoid side effects on this list!
Because letter_bank
is not a nested data structure, copy
is sufficient enough to copy over all the key-value pairs from the dictionary. deepcopy
is primarily used with nested data structures to copy over the child or inner structures. Here's more info:
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.
gotta start reading the documentation :)
for letter in word.upper(): | ||
if letter not in letter_bank_copy: | ||
return False | ||
elif letter in letter_bank_copy: | ||
letter_bank_copy.remove(letter) | ||
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.
👍
scoring_system = { | ||
1: ['A', 'E', 'I', 'O', 'U', 'L', 'N', 'R', 'S', 'T'], | ||
2: ['D', 'G'], | ||
3: ['B', 'C', 'M', 'P'], | ||
4: ['F', 'H', 'V', 'W', 'Y'], | ||
5: ['K'], | ||
8: ['J', 'X'], | ||
10: ['Q', 'Z'] | ||
} | ||
|
||
word_score = 0 | ||
|
||
for letter in word: | ||
for score, letters in scoring_system.items(): | ||
if letter.upper() in letters: | ||
word_score += score | ||
|
||
#bonus if word is between 7 to 10 characters | ||
if 7 <= len(word) <=10: | ||
word_score += 8 | ||
|
||
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.
👍 Nice work Kat!
I really like how you took advantage of utilizing int
s as keys in python dictionaries. Optionally, we could move this data structure outside of the function for improved readability.
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.
ohhhh I definitely see your point!!
for letter in word: | ||
for score, letters in scoring_system.items(): | ||
if letter.upper() in letters: | ||
word_score += 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.
Keep in mind that in
on lists (letters
) is a hidden iterator under the hood. In this case, what do you think the time complexity is for this section of code?
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.
n**2
|
||
for word in word_list: | ||
word_score = score_word(word) | ||
words_with_scores.append({word: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.
We could condense these lines by using a list comprehension to build a list of dictionaries:
words_with_scores = [ {word: score_word(word)} for word in word_list ]
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.
oohhhh I like it
|
||
for word in word_list: | ||
word_score = score_word(word) | ||
words_with_scores.append({word:word_score}) | ||
|
||
highest_score_with_word = ("", 0) | ||
|
||
for pair in words_with_scores: | ||
for word, word_score in pair.items(): | ||
if word_score >= highest_score_with_word[1] and len(word) == 10: | ||
return (word, word_score) | ||
elif word_score == highest_score_with_word[1] and len(word) < len(highest_score_with_word[0]): | ||
highest_score_with_word = (word, word_score) | ||
elif word_score > highest_score_with_word[1]: | ||
highest_score_with_word = (word, word_score) | ||
|
||
return highest_score_with_word |
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 work Kat!
Thank you Audrey !!!! |
No description provided.