zoisite - Say Ryder #113
zoisite - Say Ryder #113
Conversation
| def draw_letters(): | ||
| pass | ||
| num_letters = 10 | ||
| letters, weights = zip(*LETTER_POOL.items()) |
There was a problem hiding this comment.
Really neat use of zip! How might you explain what this line is doing to another person?
| else: | ||
| hand = random.choices(letters, weights, k = num_letters) |
There was a problem hiding this comment.
We could remove this else block without changing the outcome of the function. If we hit line 57, then valid_hand must be False, meaning we start the loop over. hand will get reassigned again on line 42 before the choices in this else can be analyzed.
| while not valid_hand: | ||
| valid_letter_count = 0 | ||
| hand_dict = {} | ||
| hand = random.choices(letters, weights, k = num_letters) |
There was a problem hiding this comment.
Love to see folks researching into functions and their optional parameters! It's extremely unlikely given the weights, but because there is no guarantee that any particular draw would be valid, in a very rare circumstance this could cause an infinite loop. Is there another way we could approach it that guarantees the function will stop looping?
| for letter in hand: | ||
| if letter in hand_dict: | ||
| hand_dict[letter] += 1 | ||
| else: | ||
| hand_dict[letter] = 1 | ||
|
|
||
| for letter in hand_dict.keys(): | ||
| if hand_dict[letter] <= LETTER_POOL[letter]: | ||
| valid_letter_count += 1 |
There was a problem hiding this comment.
I like the approach to use a frequency map to confirm we never go over the amounts in LETTER_POOL.
| 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.
| 'Y': 4, | ||
| 'Z': 10 | ||
| } | ||
| word = list(word.upper()) |
There was a problem hiding this comment.
One of the cool things about strings in python is that they are iterable - we can loop through them like a list. This means we don't need to convert word.upper() to a list unless we need to do something that requires the other abilities of lists.
| if letter in score_value_dict: | ||
| score += score_value_dict[letter] |
There was a problem hiding this comment.
Nice way to handle things like hyphenated words.
| return (words_len_ten[0], highest_word_score) |
There was a problem hiding this comment.
If we have several words tied for highest, then we should return the first of those high scores with 10 letters, if there is one. If we have input like ["AAAAAAAAAD", "JQ", "KFHK"] (In order, their scores are 11, 18, and 18), then we should still return JQ as the shortest of the highest scored words. The data in our tests didn't catch this scenario, but how could we update the function to account for it?
| score = score_word(word) | ||
| if score == highest_word_score: | ||
| if len(word) == 10: | ||
| words_len_ten.append(word) | ||
| elif len(word) < len(winning_word) and not len(winning_word) == 10: | ||
| winning_word = word | ||
| elif score > highest_word_score: | ||
| highest_word_score = score | ||
| winning_word = word | ||
| if len(word) == 10: | ||
| words_len_ten.append(word) |
There was a problem hiding this comment.
Love this approach to tiebreak in a single pass over the input!
| @@ -54,6 +54,7 @@ def test_letter_not_selected_too_many_times(): | |||
| for i in range(1000): | |||
| # Arrange/Act | |||
| letters = draw_letters() | |||
| print(letters) | |||
There was a problem hiding this comment.
We shouldn't commit changes to files that we don't intend to share out. It's all good to add code for testing or reformat things locally while we're working, but when we do, we should make sure that when we stage file for commit that we specify which files we are adding over using shortcuts that add all changed files. When we go to create a PR, we're also given a chance to review our changes before clicking "open" and that's a great place to double check that you only see the changes you intended to make.
Submitting for adagrams project feedback.