-
Notifications
You must be signed in to change notification settings - Fork 89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
C18 Yun and Intesar #76
base: master
Are you sure you want to change the base?
Conversation
Following Intersar's idea, fixed bugs. Should pass all 4 tests, too.
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 job! You've got some great code here. In the future, try to make the commits a bit more descriptive so they say what functionality was added, not just what test was passed. Nicely done with this code!
hand = [] | ||
while len(hand) < 10: | ||
random_letter = random.choice(string.ascii_uppercase) | ||
if hand.count(random_letter) < LETTER_POOL_COUNT[random_letter]: | ||
hand.append(random_letter) | ||
continue | ||
else: | ||
continue |
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.
Nice logic! We can slightly simplify things be removing lines 43-44, they are unneeded.
def uses_available_letters(word, letter_bank): | ||
pass | ||
word_case = word.upper() | ||
letter_count_dict = Counter(letter_bank) | ||
for letter in word_case: | ||
if letter in letter_count_dict and letter_count_dict[letter] > 0: | ||
letter_count_dict[letter] -= 1 | ||
continue | ||
else: | ||
return False | ||
|
||
|
||
return True |
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.
Nice use of Counter
! Can you also think through how you would solve this problem without it?
Also, the continue
on line 54 is unneeded.
word_score_dict[word] = word_score | ||
|
||
highest_word_score = max(set(score_list)) |
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.
The set
is unneeded here, max
can operate directly on a list.
if len(word) == 10: | ||
winning_tuple = (word, score) | ||
break | ||
else: | ||
winning_tuple = list(highest_word_dict.items())[0] |
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.
Line 95 repeatedly converts the items in a dictionary to a list. Can you think of a way to avoid this?
# for word in word_list: | ||
# word_score = score_word(word) | ||
# word_to_score_dict[word] = word_score |
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.
Nice score dictionary!
# for word in word_list: | ||
# word_score = score_word(word) | ||
# word_to_score_dict[word] = word_score | ||
# winning_words = [] | ||
# highest_scoring_word = () | ||
# highest_scoring_word = (word, word_score) |
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.
Line 106 will start us with the last word in the data, which is likely not what we want because earlier words should win tiebreaks.
# if highest_scoring_word == (): | ||
# highest_scoring_word = (word, word_score) | ||
# continue | ||
# if word_score > highest_scoring_word[1]: | ||
# highest_scoring_word = (word, word_score) | ||
# winning_words.append(highest_scoring_word) | ||
|
||
# if word_score == highest_scoring_word[1]: | ||
# if len(highest_scoring_word[0]) == 10: | ||
# winning_words.append(highest_scoring_word) | ||
# return highest_scoring_word | ||
|
||
# if len(word) == 10 and len(highest_scoring_word[0]) <= len(word): | ||
# highest_scoring_word = (word, word_to_score_dict[word]) | ||
# winning_words.append(highest_scoring_word) | ||
# return highest_scoring_word | ||
# if len(highest_scoring_word[0]) > len(word): | ||
# highest_scoring_word = (word, word_to_score_dict[word]) | ||
# elif len(highest_scoring_word[0]) == len(word): | ||
# highest_scoring_word = (word, word_to_score_dict[word]) | ||
|
||
# winning_words.append(highest_scoring_word) | ||
# return highest_scoring_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.
There's a lot of great logic here! I think the main issue this code has is the return statements being inside the for
loop. A return statement immediately ends our loop and exits our function, which means that none of our later words would be considered. Moving our return statement outside the loop would solve. lot here!
No description provided.