C16 - Spruce - Sandra Caballero and Vange Spracklin#46
C16 - Spruce - Sandra Caballero and Vange Spracklin#46HouseOfVange wants to merge 9 commits intoAda-C16:masterfrom
Conversation
audreyandoy
left a comment
audreyandoy
left a comment
There was a problem hiding this comment.
Great work Sandra and Vange!
Your submission covers all the learning goals and passes all the tests! This project is definitely worthy of a green grade 🟢 🌲✨
My feedback primarily focuses on ways to use guard clauses, methods from the random package, and compound conditional expressions. Overall, the code was clean and easy to read.
Keep up the great work 🌲 ✨
| @@ -1,11 +1,199 @@ | |||
| import random | |||
|
|
|||
| LETTER_POOL = ( | |||
There was a problem hiding this comment.
Great choice in using a tuple of nested dictionaries. While we encourage copying any data structure rather than altering it directly, the tuple adds an extra layer of protection.
There was a problem hiding this comment.
I also like how the tuple is stored as a constant variable to prevent clogging up other functions. Keeping the functions nice and focused!
| } | ||
| ) | ||
|
|
||
| def draw_letters(): |
| for i in range(10): | ||
| index = random.randint(0, len(letter_pool_list)-1) | ||
| letter = letter_pool_list[index] | ||
| hand_of_letters.append(letter) | ||
| letter_pool_list.pop(index) |
There was a problem hiding this comment.
Good use of pop to ensure that a letter isn't selected more than it's frequency.
There was a problem hiding this comment.
Optionally, random.sample() can be used here to generate the random letters instead of this for loop.
| for i in range(10): | |
| index = random.randint(0, len(letter_pool_list)-1) | |
| letter = letter_pool_list[index] | |
| hand_of_letters.append(letter) | |
| letter_pool_list.pop(index) | |
| hand_of_letters = random.sample(letter_pool_list, 10) |
There was a problem hiding this comment.
Here's more info on the sample in case it will come handy in the future: https://docs.python.org/3/library/random.html#random.sample
|
|
||
| def uses_available_letters(word, letter_bank): | ||
| pass | ||
| letter_bank_copy = letter_bank[:] |
There was a problem hiding this comment.
Great use of the slice operator to make a shallow copy.
| for letter in word_character_list: | ||
| if letter in letter_bank_copy: | ||
| letter_bank_copy.remove(letter) | ||
| else: | ||
| return False | ||
| return True |
There was a problem hiding this comment.
We can restructure this block by reversing the conditional logic into a guard clause. The if letter not in letter_bank_copy will become the 'guard' that checks if letter does not meet the criteria to make the loop continue. In this case, if letter is not in 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 letter in word_character_list: | |
| if letter in letter_bank_copy: | |
| letter_bank_copy.remove(letter) | |
| else: | |
| return False | |
| return True | |
| for letter in word_character_list: | |
| if letter not in letter_bank_copy: | |
| return False | |
| letter_bank_copy.remove(letter) | |
| return True |
| return False | ||
| return True | ||
|
|
||
| def score_word(word): |
| if len(word) == 10 and len(top_word) == 10: | ||
| continue | ||
| elif len(top_word) == 10: | ||
| continue |
There was a problem hiding this comment.
We can combine these two into a combined conditional expressions since they contain the same logic:
| if len(word) == 10 and len(top_word) == 10: | |
| continue | |
| elif len(top_word) == 10: | |
| continue | |
| if len(word) == 10 and len(top_word) == 10 or len(top_word) == 10: | |
| continue |
| top_word = word | ||
| top_score = score_word(word) | ||
|
|
||
| best_word = [top_word, top_score] |
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.
| best_word = [top_word, top_score] | |
| return word_result, score_count |
or
| best_word = [top_word, top_score] | |
| best_word = (top_word, top_score) |
3 participants
No description provided.