Conversation
…e and optimized for Big O
yangashley
left a comment
yangashley
left a comment
There was a problem hiding this comment.
Great work on this project Morgan and Philomena!
You both passed the tests and met the learning goals. I left a couple of small comments about naming variables/functions and optimizations.
You both have earned a 🟢 on Adagrams!
| import random | ||
| from collections import Counter | ||
|
|
||
| LETTER_POOL = { |
There was a problem hiding this comment.
I like that this defined as a "constant" outside the function. Moving a large chunk of data out from a function helps declutter the function.
And I see what you did there with the letter frequency and letter score in the list. You might even consider renaming LETTER_POOL to something like LETTER_FREQ_WITH_SCORES to indicate that this dict holds 2 different kinds of information.
|
|
||
| Returns: | ||
| returned_letters (list): list of 10 randomly chosen letters | ||
| """ |
| returned_letters = [] | ||
| for letter, number in LETTER_POOL.items(): | ||
| letter_list += letter * number[0] | ||
| for i in range(10): |
There was a problem hiding this comment.
You could also write this loop like this while len(letters) < 10:
| """ | ||
| Checks if the provided word utilizes letters only found in letter bank | ||
|
|
||
| Parameters: | ||
| word (str): the played word | ||
| letter_bank (list): bank of randomly assigned letters | ||
|
|
||
| Returns: | ||
| boolean value based on word validity | ||
| """ |
| Returns: | ||
| boolean value based on word validity | ||
| """ | ||
| letter_dict = Counter(letter_bank) |
There was a problem hiding this comment.
Great job building a frequency map from the letters to efficiently check whether we can make the supplied word!
|
|
||
| for letter in word.upper(): | ||
| try: | ||
| if letter in letter_dict.keys() and letter_dict[letter] > 0: |
There was a problem hiding this comment.
Take care to use key in dict syntax. If you dokey 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.
if letter in letter_dict and letter_dict[letter] > 0:
|
|
||
| return True | ||
|
|
||
| def score_word(word): |
| Calculates the highest score in a list of words. | ||
|
|
||
| Analyzes high scores to check for a tie. | ||
| Follows game rules for tie breaker: | ||
|
|
||
| Ties are judged based off of letter count: | ||
|
|
||
| If Their is a 12 letter word present, returns the | ||
| first 12 letter word that appears in the list. | ||
|
|
||
| For cases without a 12 letter word, returns the | ||
| shortest word in the list. | ||
|
|
||
| If the shortest words are the same length, return | ||
| the one that appears first in the original list. |
There was a problem hiding this comment.
There aren't hard and fast rules about writing docstrings. Some devs, myself included, prefer shorter docstrings where possible. I think calling out the input and outputs suffices since I can gather how ties are handled by reading the code. I think that you could leave this first chunk out and I'd still understand what this function does
| for word, top_score in scores: | ||
| word_length = len(word) | ||
| if word_length == 10: | ||
| return word, top_score | ||
| elif not lowest: | ||
| lowest = (word, top_score, word_length) | ||
| elif word_length < lowest[2]: | ||
| lowest = (word, top_score, word_length) |
There was a problem hiding this comment.
You could also pull this into a helper method to simplify the logic in this function
2 participants
No description provided.