Tiger - Lily & Stacy #59
Conversation
There was a problem hiding this comment.
Great job working together, Lily and Stacy! Your solution was very creative!
Here are a few things to consider for future projects that I've detailed below in your code:
- When looking up values that have a relationship ("A" has a quantity of 9), it is likely more efficient to use a dictionary, rather than lists or lists inside a dictionary. That's because a list has a lookup time of O(n), whereas a dictionary always has a lookup time of O(1).
If you have any questions about the feedback, please reach out to me!
adagrams/game.py
Outdated
| def draw_letters(): | ||
| pass | ||
| distribution_of_letters = {"A" : 9, "N" : 6, "B" : 2, "O" : 8, "C" : 2, "P" : 2, "D" : 4 ,"Q" : 1, "E" : 12, "R" : 6, "F" : 2, "S" : 4, "G" : 3, "T" : 6, "H" : 2, "U" : 4, "I" : 9, "V" : 2, "J" : 1 , "W" : 2, "K" : 1, "X" : 1, "L" : 4, "Y" : 2, "M" : 2, "Z" : 1} |
There was a problem hiding this comment.
Consider declaring this as a global "constant" (maybe POOL_OF_LETTERS) outside this function.
Doing so wouldn't really save on runtime (since we would then need to modify the counts and make a copy at the start of each call to this method), but it does declutter the method a bit by allowing us to move the large data structure to some innocuous corner of our file.
| userletters = random.choice(list(distribution_of_letters.items())) | ||
| if userletters[1] > 0: | ||
| letters.append(userletters[0]) | ||
| distribution_of_letters[userletters[0]]-=1 |
There was a problem hiding this comment.
Oh what an interesting way to handle checking the letter and the quantity!
| if userletters[1] > 0: | ||
| letters.append(userletters[0]) | ||
| distribution_of_letters[userletters[0]]-=1 | ||
| return letters | ||
|
|
||
| def uses_available_letters(word, letter_bank): |
adagrams/game.py
Outdated
| if len(word) != 0: | ||
| special_length = [7, 8, 9, 10] | ||
|
|
||
| # add additional 8 points for words with special length | ||
| if len(word) in special_length: | ||
| total_score += 8 |
There was a problem hiding this comment.
Very creative! Consider refactoring this so we don't need a new data structure on line 51 and a for loop on line 54 (under the hood, if...in with a list is a loop).
We could combine the if statements together, like:
if len(word) > 6 and len(word) < 11:
total_score += 8
adagrams/game.py
Outdated
| for point, letters in score_chart.items(): | ||
| for letter in word.upper(): | ||
| if letter in letters: | ||
| total_score += point |
There was a problem hiding this comment.
this works just fine! One thing to consider: looking up an item in a list is generally O(n), but the lookup time for a dictionary is O(1).
If we kept our dictionary like: letter_dict = {"A": 1, "B": 2....}, then our look up will be constant, and that will get rid of one of the the three for loops in this code block.
for letter in word.upper():
if letter in letter_dict:
# increase your total_score herenow our code only has one for loop in it!
| best_word = (word, score) |
There was a problem hiding this comment.
This works! We could also do this on line 67 initially:
best_word = (word_list[0], score_word(word_list[0]))
adagrams/game.py
Outdated
|
|
||
| # iterate through the word_list, and update best_word when current word is the best word | ||
| for word in word_list: | ||
| score = score_word(word) |
There was a problem hiding this comment.
It's great that you have comments here to tell other coders what's going on, or to remind you later what this code does.
But with so many comments cutting up the code, it actually makes it harder to read. So here's what we can do:
def get_highest_word_score(word_list):
'''
Function iterates through word_list, and updates best_word
when current word is the best word.
The rest of your comments here
'''
best_word = None
# the rest of your code hereWe can bring all our comments up to the top inside docstrings
adagrams/game.py
Outdated
| point_1_letters = ["A", "E", "I", "O", "U", "L", "N", "R", "S", "T"] | ||
| point_2_letters = ["D", "G"] | ||
| point_3_letters = ["B", "C", "M", "P"] | ||
| point_4_letters = ["F", "H", "V", "W", "Y"] | ||
| point_5_letters = ["K"] | ||
| point_8_letters = ["J", "X"] | ||
| point_10_letters = ["Q", "Z"] | ||
|
|
||
| # use map to store letter to score relationships | ||
| score_chart = { | ||
| 1: point_1_letters, | ||
| 2: point_2_letters, | ||
| 3: point_3_letters, | ||
| 4: point_4_letters, | ||
| 5: point_5_letters, | ||
| 8: point_8_letters, | ||
| 10: point_10_letters | ||
| } |
There was a problem hiding this comment.
This takes a lot of lines. Let's combine this in one:
score_chart = {
1: ["A", "E", "I", "O", "U", "L", "N", "R", "S", "T"]
...
Finished the project and passed all tests.