C19 Sapphire - Linh Huynh #116
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,170 @@ | ||
| import random | ||
| from collections import Counter | ||
|
|
||
| LETTER_POOL = { | ||
| 'A': 9, | ||
| 'B': 2, | ||
| 'C': 2, | ||
| 'D': 4, | ||
| 'E': 12, | ||
| 'F': 2, | ||
| 'G': 3, | ||
| 'H': 2, | ||
| 'I': 9, | ||
| 'J': 1, | ||
| 'K': 1, | ||
| 'L': 4, | ||
| 'M': 2, | ||
| 'N': 6, | ||
| 'O': 8, | ||
| 'P': 2, | ||
| 'Q': 1, | ||
| 'R': 6, | ||
| 'S': 4, | ||
| 'T': 6, | ||
| 'U': 4, | ||
| 'V': 2, | ||
| 'W': 2, | ||
| 'X': 1, | ||
| 'Y': 2, | ||
| 'Z': 1 | ||
| } | ||
|
|
||
| SCORE_CHART = { | ||
| 'A': 1, | ||
| 'B': 3, | ||
| 'C': 3, | ||
| 'D': 2, | ||
| 'E': 1, | ||
| 'F': 4, | ||
| 'G': 2, | ||
| 'H': 4, | ||
| 'I': 1, | ||
| 'J': 8, | ||
| 'K': 5, | ||
| 'L': 1, | ||
| 'M': 3, | ||
| 'N': 1, | ||
| 'O': 1, | ||
| 'P': 3, | ||
| 'Q': 10, | ||
| 'R': 1, | ||
| 'S': 1, | ||
| 'T': 1, | ||
| 'U': 1, | ||
| 'V': 4, | ||
| 'W': 4, | ||
| 'X': 8, | ||
| 'Y': 4, | ||
| 'Z': 10 | ||
| } | ||
|
|
||
| LETTER_POOL_LIST = [] | ||
| temp_list = [] | ||
|
|
||
| def draw_letters(): | ||
|
Comment on lines
+62
to
-2
There was a problem hiding this comment. Your solution would benefit if we moved these two variables to inside the def draw_letters():
LETTER_POOL_LIST = []
temp_list = []
# ...The first sign that we should move these variables is because they are only used/modified/accessed within one function: the As a note, if we move these variables, all of your tests pass! Please let me know if you'd like to talk together about why this fix works. |
||
|
|
||
| # Creates a comprehensive list which includes the total quanity of each available letter from the LETTER_POOL dictionary as individual elements. | ||
| for letter, quantity in LETTER_POOL.items(): | ||
| temp_list.append([letter] * quantity) | ||
|
|
||
| for sublist in temp_list: | ||
| for letter in sublist: | ||
|
Comment on lines
+71
to
+72
There was a problem hiding this comment. For me, it's hard to understand what |
||
| LETTER_POOL_LIST.append(letter) | ||
|
|
||
| # Chooses 10 random letters as hand, excluding the ones that are already chosen. | ||
| #hand = random.sample(LETTER_POOL_LIST, 10) | ||
|
|
||
| hand = [] | ||
| while len(hand) < 10: | ||
| random_letter = random.choice(LETTER_POOL_LIST) | ||
| random_letter_index = LETTER_POOL_LIST.index(random_letter) | ||
| LETTER_POOL_LIST.pop(random_letter_index) | ||
| hand.append(random_letter) | ||
|
|
||
| return hand | ||
|
|
||
| draw_letters() | ||
|
There was a problem hiding this comment. This line doesn't do anything meaningful right now, since it's not in a test file or a function! This line is safe to delete. |
||
|
|
||
| def uses_available_letters(word, letter_bank): | ||
| word_verification = [] | ||
| hand_letter_count = {} | ||
| word_letter_count = {} | ||
| count = 0 | ||
| capitalized_word = word.upper() | ||
|
|
||
| for letter in letter_bank: | ||
| if letter not in hand_letter_count: | ||
| count = 1 | ||
| hand_letter_count[letter] = count | ||
| else: | ||
| count += 1 | ||
| hand_letter_count[letter] = count | ||
|
Comment on lines
+96
to
+102
There was a problem hiding this comment. Your logic is sound here, but there are a couple of ways we can make it clearer by focusing on for letter in letter_bank:
count = 0
if letter not in hand_letter_count:
count = 1
hand_letter_count[letter] = count
else:
count += 1
hand_letter_count[letter] = countAfterwards, we can really see how for letter in letter_bank:
if letter not in hand_letter_count:
hand_letter_count[letter] = 1
else:
hand_letter_count[letter] += 1This code has the exact same logic as your original project, and three less lines and one less variable! |
||
|
|
||
| for letter in capitalized_word: | ||
| letter_quantity = capitalized_word.count(letter) | ||
|
Comment on lines
+104
to
+105
There was a problem hiding this comment. Reinforcing a comment I've made, |
||
| word_letter_count[letter] = letter_quantity | ||
|
|
||
| if letter in letter_bank and word_letter_count[letter] <= hand_letter_count[letter]: | ||
| word_verification.append("True") | ||
| else: | ||
| word_verification.append("False") | ||
|
Comment on lines
+109
to
+111
There was a problem hiding this comment.
|
||
|
|
||
| if "False" in word_verification: | ||
| return False | ||
| else: | ||
| return True | ||
|
Comment on lines
+113
to
+116
There was a problem hiding this comment. This logic says "If we've come across even one False is inside We can move I'd encourage you to work through this refactoring and see if any questions come up. After refactoring that, I think it's worth asking "Do I need the |
||
|
|
||
|
|
||
| def score_word(word): | ||
| word_score = {} | ||
|
|
||
| if word == "": | ||
| word_score["empty"] = 0 | ||
|
There was a problem hiding this comment. [nit] Your indentation on this line is off by one space
Comment on lines
+122
to
+123
There was a problem hiding this comment. This code says to me "if if word == "":
return 0 |
||
|
|
||
| for letter in word.upper(): | ||
| if letter in SCORE_CHART: | ||
| if letter not in word_score: | ||
| word_score[letter] = SCORE_CHART[letter] | ||
| else: | ||
| word_score[letter] += SCORE_CHART[letter] | ||
|
|
||
| if 7 <= len(word) <= 10: | ||
| word_score["bonus"] = 8 | ||
|
|
||
| return sum(word_score.values()) | ||
|
Comment on lines
+120
to
+135
There was a problem hiding this comment. I think your approach to this problem is interesting and extensible-- I think that adding |
||
|
|
||
| def get_highest_word_score(word_list): | ||
| score_dict = {} | ||
| score_list = [] | ||
| matching_scores_dict = {} | ||
| highest_score = 0 | ||
|
|
||
| for word in word_list: | ||
| score = score_word(word) | ||
| score_dict[word] = score | ||
|
|
||
| for word, score in score_dict.items(): | ||
| score_list.append(score) | ||
| if score > highest_score: | ||
| highest_score = score | ||
| winning_word = word | ||
|
|
||
| matching_scores = [k for k,v in Counter(score_list).items() if v>1] | ||
|
|
||
| for word, score in score_dict.items(): | ||
| if score in matching_scores: | ||
| matching_scores_dict[word] = score | ||
|
Comment on lines
+153
to
+157
There was a problem hiding this comment. From what I can tell, this code is finding all times that there is a non-unique score, and collects them in a dictionary between word and score. However, do we need to keep a dictionary of matching scores to find the best word? |
||
|
|
||
| for word, score in matching_scores_dict.items(): | ||
|
There was a problem hiding this comment. Instead of iterating through If we refactor this loop so it iterates through |
||
| if score == highest_score and len(word) == 10: | ||
| highest_score = score | ||
| winning_word = word | ||
| break | ||
| elif score == highest_score and len(word) < len(winning_word): | ||
| highest_score = score | ||
| winning_word = word | ||
| break | ||
|
|
||
| best_word = (winning_word, highest_score) | ||
| return best_word | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great call making these constants