Spruce - Mary Tian #52
Conversation
To update the repository
…ble names for clarity in score_word function
… function as well
There was a problem hiding this comment.
Great work Mary and Rebecca!
Your submission covers all the learning goals and passes all the tests! Overall, the code was clean and easy to read. This project is definitely worthy of a green grade 🟢 🌲✨
My feedback primarily focuses on ways to use guard clauses, using const variables, and syntax for iterating through dictionaries. I also loved all the docstrings under each function, they were definitely helpful in understanding your thought process for these solutions.
Keep up the great work 🌲 ✨
| 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.
Long chunks of data like letter_bank tend to clutter functions and require more scrolling to see the rest of the function body. Consider moving this dictionary outside of this function (or in a new file) as a constant variable to be referenced in draw_letters.
| '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 | ||
| } | ||
| letter_to_draw = list(LETTER_POOL.keys()) |
There was a problem hiding this comment.
| letter_to_draw = list(LETTER_POOL.keys()) | |
| letter_to_draw = list(LETTER_POOL) |
We can omit keys() as the default iterator for dictionaries are its own keys. This means the list function will iterate through LETTER_POOL and create a list of its keys.
| for letter in random.sample(letter_to_draw, 10): | ||
| if letter not in letters_drawn: | ||
| letters_drawn.append(letter) | ||
| letter_count[letter] = 1 | ||
| elif letter_count[letter] < LETTER_POOL[letter]: | ||
| letters_drawn.append(letter) | ||
| letter_count[letter] += 1 |
There was a problem hiding this comment.
Love the use of a frequency map to keep track of the letters being drawn!
There was a problem hiding this comment.
We could also use sample to build letters_drawn directly from LETTER_POOL.
letters_drawn = random.sample(LETTER_POOL.keys(), counts=LETTER_POOL.values(), k=10)Here's the sample documentation, if you'd like to learn more about the method.
| for letter in word: | ||
| if letter not in letter_bank or word.count(letter) > letter_bank.count(letter): | ||
| is_valid = False | ||
| else: | ||
| is_valid = True | ||
|
|
||
| return is_valid |
There was a problem hiding this comment.
Great use of a guard clause here. We can make the code even neater by getting rid of the else clause (aka the dangly bit). To do so, we can take advantage of using the is_valid flag to describe the single scenario where is_valid would be false in the guard clause. Otherwise, the function will just return True. Like so:
is_valid = True
for letter in word:
if letter not in letter_bank or word.count(letter) > letter_bank.count(letter):
is_valid = False
return is_valid
| for letter in word: | |
| if letter not in letter_bank or word.count(letter) > letter_bank.count(letter): | |
| is_valid = False | |
| else: | |
| is_valid = True | |
| return is_valid | |
| is_valid = True | |
| for letter in word: | |
| if letter not in letter_bank or word.count(letter) > letter_bank.count(letter): | |
| is_valid = False | |
| return is_valid |
| LETTER_POINTS = { | ||
| 'A': 1, 'B': 3, 'C': 3, 'D': 2, 'E': 1, 'F': 4, 'G': 2, 'H': 4, 'I': 1, 'J': 8, | ||
| 'K': 5, 'L': 1, 'M': 3, 'N': 1, 'O': 1, 'P': 3, 'Q': 10, 'R': 1, 'S': 1, 'T': 1, 'U': 1, | ||
| 'V': 4, 'W': 4, 'X': 8, 'Y': 4, 'Z': 10 | ||
| } |
There was a problem hiding this comment.
Similar to my feedback on LETTER_POOL, we can declutter this function by moving LETTER_POINTS outside of the function as a const variable.
| 'V': 4, 'W': 4, 'X': 8, 'Y': 4, 'Z': 10 | ||
| } | ||
| bonus_range = [7, 8, 9, 10] | ||
| total = sum([LETTER_POINTS[letter] for letter in word.upper()]) |
There was a problem hiding this comment.
LOVELY use of list comprehension here!
| if len(word) in bonus_range: | ||
| total += 8 | ||
|
|
||
| return total |
There was a problem hiding this comment.
This function is so neat, great work!
| largest_word_len = max((len(key) for key in words_scored_dict.keys())) |
There was a problem hiding this comment.
keys() can be removed as the default iterator for dicts are its own keys.
| smallest_word_len = min((len(key) for key in words_scored_dict.keys())) | |
| largest_word_len = max((len(key) for key in words_scored_dict.keys())) | |
| smallest_word_len = min((len(key) for key in words_scored_dict.keys())) | |
| largest_word_len = max((len(key) for key in words_scored_dict.keys())) |
There was a problem hiding this comment.
Really cool use of dictionary and list comprehensions!!
| continue | ||
| elif len(user_word) == 10: | ||
| winning_word = user_word | ||
| highest_word_score = score | ||
| break |
There was a problem hiding this comment.
We can take out if len(user_word) != 10 as the loop will continue anyway.
| if len(user_word) != 10: | |
| continue | |
| elif len(user_word) == 10: | |
| winning_word = user_word | |
| highest_word_score = score | |
| break | |
| if len(user_word) == 10: | |
| winning_word = user_word | |
| highest_word_score = score | |
| break |
| winning_word = user_word | ||
|
|
||
| return winning_word, highest_word_score |
No description provided.