Conversation
yangashley
left a comment
yangashley
left a comment
There was a problem hiding this comment.
Great work on this project Sujin and Danielle!
You both passed the tests and met the learning goals. I left some comments on variable names and suggested an optimization you could make.
You both have earned a 🟢 on Adagrams!
| 'X': 8, | ||
| 'Y': 4, | ||
| 'Z': 10 | ||
| } |
There was a problem hiding this comment.
I like that these are defined as "constants" outside the function. Moving large chunks of data out from a function helps declutter the function.
|
|
||
| user_letters = [] | ||
| copy_letter_pool = dict(LETTER_POOL) | ||
| print(copy_letter_pool) |
There was a problem hiding this comment.
You can remove debugging print statements before submitting your project to keep your functions clean.
| copy_letter_pool = dict(LETTER_POOL) | ||
| print(copy_letter_pool) | ||
| while len(user_letters) < 10: | ||
| random_letter = random.choice(list(copy_letter_pool.keys())) |
There was a problem hiding this comment.
Calling list() on a dictionary takes all the keys and puts them in a list so we get a list of 26 letters here.
In order to select a random letter from a pool that reflects what you have declared in LETTER_POOL, you could create a list pool_of_letters and use the keys and values from LETTER_POOL to populate a list with 9 As, 2 Bs, 2 C,s etc.
pool_of_letters = []
for letter, number in LETTER_POOL.items():
pool_of_letters += letter * number
# You could then do user `random.choice(pool_of_letters)` This is just one approach to populating pool_of_letters, feel free to use any other approach that comes to mind too.
| user_letters.append(random_letter) | ||
| copy_letter_pool.update({random_letter:copy_letter_pool[random_letter] - 1}) # Reduce value of letter by 1 | ||
| if copy_letter_pool[random_letter] == 0: # Delete any letter from dict with value of 0 | ||
| del copy_letter_pool[random_letter] |
There was a problem hiding this comment.
You can get around having to delete from key-value deleting by adding a check a little earlier in this function.
Adding a check after line 68 to check if the value of copy_letter_pool is greater than 0 before appending random_letter to user_letters makes it so you wouldn't need to delete anything from the dictionary.
|
|
||
| def uses_available_letters(word, letter_bank): | ||
| pass | ||
| aCopy = list(letter_bank) |
There was a problem hiding this comment.
Function names should be lowercase, with words separated by underscores to adhere to the Python style guide: readability.https://peps.python.org/pep-0008/#function-and-variable-names
| pass | ||
| aCopy = list(letter_bank) | ||
| for letter in word: | ||
| if letter.upper() in aCopy: |
There was a problem hiding this comment.
The in operator on lists has linear time complexity on top of the first loop that's running on loop 79. You can optimize this here by using letter_bank to create a frequency dictionary where the keys are the letters and the values are the frequency of the letter. For example: letter_frequency = {"a": 1, "e": 2}
Then when you check if a letter is a key of letter_frequency, you reduce your time complexity to O(1).
for letter in word:
if letter in letter_frequency:Take care to use key in dict syntax. If you do key in dict.keys() you'd reproduce O(n) time complexity since under the hood Python creates a list of keys when you call .keys() and the in operator for lists is linear.
| return True | ||
|
|
||
|
|
||
| def score_word(word): |
| elif score_word(word) == highestScore: | ||
| if len(word) == 10 and len(winningWord) != 10: | ||
| winningWord = word | ||
| if len(word) < len(winningWord) and len(winningWord) == 10: | ||
| winningWord != word | ||
| elif len(word) < len(winningWord): | ||
| winningWord = word |
There was a problem hiding this comment.
You could also pull this into a helper function to help simplify the logic in this function overall.
| highestScore = 0 | ||
| winningWord = "" |
There was a problem hiding this comment.
Python uses snake_case for naming variables so these could be renamed to highest_score and winning_word.
3 participants
No description provided.