Spruce - Zandra & Reid #44
Conversation
…gs. Complete and ready for submission.
There was a problem hiding this comment.
Great work Zandra and Reid!
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 declutter functions, copy data structures, and use guard clauses. 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.
| letter_bank_copy = [] | ||
| for item in letter_bank: | ||
| letter_bank_copy.append(item) |
There was a problem hiding this comment.
We can eliminate the for loop and reduce these 3 lines into 1 by using the copy method to generate a copied list from letter_bank.
| letter_bank_copy = [] | |
| for item in letter_bank: | |
| letter_bank_copy.append(item) | |
| letter_bank_copy = letter_bank.copy() |
| for char in word: | ||
| if char in letter_bank_copy: | ||
| letter_bank_copy.remove(char) | ||
| else: | ||
| return False |
There was a problem hiding this comment.
We can restructure this block by reversing the conditional logic into a guard clause. The if char not in letter_bank_copy is the 'guard' that checks if char does not meet the criteria to make the loop continue. In this case, if char is not in the letter_bank_copy list then the loop will exit early and return False.
As Ansel describes, the guard clause removes the need for dangly bits like the else clause.
| for char in word: | |
| if char in letter_bank_copy: | |
| letter_bank_copy.remove(char) | |
| else: | |
| return False | |
| for char in word: | |
| if char not in letter_bank_copy: | |
| return False | |
| letter_bank_copy.remove(char) |
| return True | ||
|
|
||
| # Wave 3 | ||
|
|
||
|
|
||
| def score_word(word): |
There was a problem hiding this comment.
👍 Nice job! The dictionary makes this solution neat. To make this function even neater, try moving score_chart outside the function as a constant variable.
| word_result = given_word | ||
|
|
||
| return [word_result, score_count] |
There was a problem hiding this comment.
Our tests don't check for the return type, but the word and the score should be returned as a tuple rather than a list. Tuples, like lists, also use indexing to access values which is why your solution passed the tests.
| return [word_result, score_count] | |
| return word_result, score_count |
or
| return [word_result, score_count] | |
| return (word_result, score_count) |
| random.shuffle(letter_list) | ||
|
|
||
| return letter_list[0:10] |
There was a problem hiding this comment.
Really cool combo of the shuffle method and slicing to generate a random hand of letters!
|
|
||
| # Wave 1 | ||
|
|
||
|
|
||
| def draw_letters(): |
| return score | ||
|
|
||
| # Wave 4 | ||
|
|
||
|
|
||
| def get_highest_word_score(word_list): |
There was a problem hiding this comment.
👍 The logic for this function looks great! It's really easy to get bogged down into unnecessary nested conditionals for this function so great work in keeping the conditionals very neat.
No description provided.