Sea Turtles - Adagrams - Hope Wilson & Jae Clayton#56
Sea Turtles - Adagrams - Hope Wilson & Jae Clayton#56hopeolaide wants to merge 33 commits intoada-c17:masterfrom
Conversation
…ng thing, so i tried idea of making a list of just words that have winning score level ?
…ll gotta work on min
anselrognlie
left a comment
anselrognlie
left a comment
There was a problem hiding this comment.
Nice job!
All waves are complete and all tests are passing! Your code is well organized, and you are generally using good, descriptive variable names. You used lists and dictionaries throughout, and were aware of when you needed to make copies to avoid side-effects.
The main thing I'd be careful of is doing extra looping over data or performing repeat calculations in a way that could impact the efficiency of our code. There were a couple places where an extra loop was being done, or looping was used where a key lookup would do, or we were doing looping within a loop resulting in O(n^2) complexity. Sometimes building a helper data structure before starting our main work can let us avoid inner looping!
Your tile picking function does always result in valid hands, but the probability is a little different from what we'd get if we were literally drawing tiles from a pile. Try thinking about the steps you would perform in real life and try expressing those in code.
Overall, great work!
| # if word_score == highest_score: | ||
| # best_word.append(word) |
There was a problem hiding this comment.
I think one thing you'd want to do is keep track of what the last maximum score was. Then, if you find a new maximum score, you could clear out the words that were previously in the best word list.
| @@ -0,0 +1,153 @@ | |||
| """WAVE 4 Old Code Review """ | |||
|
|
|||
| """ Everything below is from the previous version that got us to 4 passed tests. """ | |||
There was a problem hiding this comment.
The main issue I see is not clearing out the previous max words when you find a new max score. But otherwise I think this approach all looks good!
| import random | ||
| import copy | ||
|
|
||
| LETTER_POOL = { |
There was a problem hiding this comment.
👍 I do like having these giant data structures not cluttering up a function. Naming the variable like a constant helps us communicate that we should be careful not to do anything that would alter the data.
| 'Y': 2, | ||
| 'Z': 1 | ||
| } | ||
| SCORE_CHART = { |
There was a problem hiding this comment.
Nice job restructuring this data from how it was presented in the project description. Organizing the score chart this way will help us take advantage of a piece of data that we have (a letter in a word) to get its score very efficiently with a dictionary lookup.
| } | ||
| def draw_letters(): | ||
| pass | ||
| user_pool = copy.deepcopy(LETTER_POOL) |
There was a problem hiding this comment.
LETTER_POOL is a dictionary where the keys are immutable strings (they must be immutable to be keys) and the values are ins, which are also immutable. While your approach does require making a copy, since you are decrementing the counts, since everything in the dict is immutable, a shallow copy would be sufficient.
| best_score_list =[] | ||
|
|
||
| # Translating word list and scores into a dictionary and finding highest word score in dictionary: | ||
| for word in word_list: |
There was a problem hiding this comment.
We don't need this loop. This will end up re-building the whole word dictionary and finding the max score from those values as many times as there are words in the word_list. Remove this line and unindent the body.
| for word in word_score_dict: | ||
| if word_score_dict[word] == highest_word_score: | ||
| best_word_list.append(word) | ||
| best_score_list.append(word_score_dict[word]) |
There was a problem hiding this comment.
Do we need to track this? After all, every word here should have a score equal to highest_word_score.
|
|
||
| #Conditional check to see if length is more than 1 then apply tie-breaker logic | ||
| # (see helper function further below) | ||
| best_word = best_word_list[0] if len(best_word_list) == 1 else tie_breaker(best_word_list) |
There was a problem hiding this comment.
👍 Tie breaker helper function! And nice use of a ternary expression.
| return (best_word, highest_word_score) | ||
|
|
||
| #Tie-breaker logic | ||
| def tie_breaker(best_word_list): |
There was a problem hiding this comment.
👍
As soon as we find a word with ten letters, it wins. Otherwise we take the shortest.
| if len(word) == 10: | ||
| return word | ||
|
|
||
| return min(best_word_list, key = len) |
There was a problem hiding this comment.
This will find the "minimum" word by comparing the results of calling len on each word in the list. Make sure you feel comfortable with how this works (like the idea of passing functions to other functions) before using this. Do you feel like you could write a similar function yourself?
3 participants
Adagrams project complete.