completed 4 functions and tested each wave, passed all waves #24
Conversation
| @@ -1,11 +1,95 @@ | |||
| import random | |||
|
|
|||
| LETTER_POOL = { | |||
There was a problem hiding this comment.
Since LETTER_POOL is only accessed by the draw_letters function, it would be reasonable to place the dictionary inside the function rather than as a constant. There are tradeoffs, the structure would clutter the function some, but it keeps the data as close as possible to where it's being used, and would mean other functions couldn't access it to accidentally alter the contents.
| def draw_letters(): | ||
| pass | ||
| letter_list = [] | ||
| for letter, letter_count in LETTER_POOL.items(): |
There was a problem hiding this comment.
Great use of the .items() dictionary method!
| for i in range(letter_count): | ||
| letter_list.append(letter) |
There was a problem hiding this comment.
They are all about the same complexity under the hood, but there are some other options for filling a list like:
for letter, letter_count in LETTER_POOL.items():
letter_list.extend(letter * letter_count)
# Another option
for letter, frequency in LETTER_POOL.items():
letter_list += letter * frequencyThere are some interesting quirks to creating python lists with the * operator that aren't an issue here, but can cause unexpected behavior if trying to use it to create lists of lists. More info about that here if you're interested: https://nishanthjkumar.com/Strange-Python-List-Multiplication-Behavior/
| for i in range(letter_count): | ||
| letter_list.append(letter) | ||
| hand = [] | ||
| for draw in range(10): |
There was a problem hiding this comment.
The draw variable created in the loop definition never gets used, it isn't a lot, but we are allocating space for a variable that we don't need. This can be a sign that we want to look at using a different kind of loop. In this case we know that we want to keep looping while our hand has less than 10 elements - we can translate this directly into a while loop's conditional statement! What could that look like?
| hand_pick = random.randint(0, len(letter_list) -1) | ||
| hand.append(letter_list[hand_pick]) | ||
| letter_list.pop(hand_pick) |
There was a problem hiding this comment.
Nice algorithm to ensure a letter gets picked every iteration and we can't choose the same entry twice!
| for letter in upper_case_word: | ||
| if letter in copy_letter_bank: | ||
| copy_letter_bank.remove(letter) | ||
| else: | ||
| return False |
There was a problem hiding this comment.
Thinking about complexity, the outer loop will iterate over the full length of word (O(n) operation). For each element of word the statement if letter in copy_letter_bank may need to iterate the full length of letter_bank to search for letter (O(n) operation). If the letter is found, another full traversal may be necessary to shift elements when we call copy_letter_bank.remove(letter).
In a worst-case scenario where word is the same length as letter_bank this means the nested for-loop has an overall O(n * 2n) which becomes O(n^2) time complexity when we drop the constants. One way we could un-nest these loops and reduce the time complexity is with frequency maps.
If we first create a frequency map of our letter_bank, how might we use that and change up our approach to reduce our time complexity?
| if letter in copy_letter_bank: | ||
| copy_letter_bank.remove(letter) | ||
| else: | ||
| return False |
There was a problem hiding this comment.
I like the early exit as soon as we find something invalid!
| total_score = 0 | ||
|
|
||
| for letter in word: | ||
| total_score += LETTER_SCORES.get(letter, 0) |
There was a problem hiding this comment.
Great use of get to return 0 for invalid characters.
| best_score = 0 | ||
|
|
||
| for word in word_list: |
There was a problem hiding this comment.
Great approach to tie break in a single loop over word_list!
| best_word = word | ||
| elif len(word) < len(best_word) and len(best_word) != 10: | ||
| best_word = word |
There was a problem hiding this comment.
Both sides of the if/elif have the same contents best_word = word. Repeating code like this is often a sign that we should look at our code to see if there are ways we could combine our statements to reduce repetition. Thinking about long-term code maintenance, if we have to change that code in the future, it goes quicker and is less error-prone when there is only one line to update, rather than several lines that we need to keep in sync.
One of many options could be to create variables for the boolean statements:
is_new_word_ten = len(word) == 10 and len(best_word) != 10
is_new_word_smaller = len(word) < len(best_word) and len(best_word) != 10
if is_new_word_ten or is_new_word_smaller:
best_word = word
Ada Phoenix C22 - Tatyana Venanzi