Skip to content
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.

Sign up for GitHub

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

Kunzite - Danica S #124

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Kunzite - Danica S #124

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cc048d5
Pass wave 1 tests
dsarm-edu Mar 23, 2023
9e72501
pass wave 2
dsarm-edu Mar 23, 2023
bad7162
Pass wave 3
dsarm-edu Mar 23, 2023
641a957
pass wave 4
dsarm-edu Mar 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 124 additions & 4 deletions adagrams/game.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,131 @@
import random
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
}

def draw_letters():
pass
big_list = []
for letter, letter_quantity in LETTER_POOL.items():
big_dict = letter * letter_quantity
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice use of repetition to handle including multiple cpoies of a letter. Since your objective was to eventually add this to a list, consider

        letters = [letter] * letter_quantity

which will make a list rather than a string. This can then be added to your growing list as

    big_list.extend(letters)
    # or
    big_list += letters

for single_char in big_dict:
big_list.append(single_char)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider moving the first part of this function to a helper function that builds an expanded list from a dictionary of keys with counts.

draw_of_10_letters = []
while len(draw_of_10_letters) < 10:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using a for loop. Since you built an explanded list, you know every pick will be "successful". So we know this will loop exactly 10 times.

random_letter = random.choice(big_list)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When using a library function like this that we haven't discussed, I encourage you to either include a comment that explains your understanding of how it works, or consider implementing the logic yourself rather than using the library function for additional practice!

draw_of_10_letters.append(random_letter)
big_list.remove(random_letter)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we start to discuss big O (coming up) we'll see that removing from a list is realtivelty inefficient. This does guarantee that future picks won't use a letter we've already "consumed", but in a loop, we'd like to avoid remove if possible.

return draw_of_10_letters

draw_letters()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a stray function call (maybe used for testing?). We can remove this.


def uses_available_letters(word, letter_bank):
pass

word = word.upper()
for letter in word:
word_count = word.count(letter)
letter_bank_count = letter_bank.count(letter)
Comment on lines +51 to +52
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each of these calls to count requires iterating over the data to tally up the number of occurences of the letter, concealing additional inner loops that must run each time the outer loop runs. For the size of the data, this won't be an issue, but we could think about whether there's a way to count up all the letters in the word and bank in a single pass. What data structures could we use to track the count of each letter?

if letter not in letter_bank:
return False
Comment on lines +53 to +54
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The the letter weren't in the bank, its count would be 0, which will definitely be less than its count in the word (which must be at least 1, else we wouldn't have encountered it during the iteration). As a result, we can do without this check.

Performing an in or not in check for a value in a list also requires iterating through the values in the list, so this is concealing an additional inner loop that runs each time through the outer loop, which is another good reason to leave it out.

if word_count > letter_bank_count:
return False
return True

def score_word(word):
pass

score_chart = {
**dict.fromkeys(["A", "E", "I", "O", "U", "L", "N", "R", "S", "T"], 1),
**dict.fromkeys(["D", "G"], 2),
**dict.fromkeys(["B", "C", "M", "P"], 3),
**dict.fromkeys(["F", "H", "V", "W", "Y"], 4),
**dict.fromkeys(["K"], 5),
**dict.fromkeys(["J", "X"], 8),
**dict.fromkeys(["Q", "Z"], 10),
**dict.fromkeys([""], 0)
}
Comment on lines +61 to +70
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clever use of he dictionary spread ** to build up the score entries with a minimum of repetition. Note that, like the LETTER_POOL, this could live externally to the function (globally) so that it doesn't need to be regenerated with every call to the function.

We might still consider listing out the values explicitly. While there happens to be some score repetition in this case, in the general case, each tile might have its own score. And if we need to adjust the scores in the future, if they were already separated out into individual entries, we'd have less chance of affecting another tile. Currently, if we adjust a tile score, we need to remove it from an existing group, and either add it to another group, or create a new group (with the potential to affect the scorign of other tiles).


word = word.upper()
total_score = 0
for letter in word:
if word == "":
return False
Comment on lines +75 to +76
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If word is empty, we'll never reach this check (the iteration will never occur). We can remove this check.

Further, there was no specification to indicate that we should return false. There's a test that checks for an empty word resulting in a score of 0.

Generally, we should return a consistent type from a function. It's surprising for a dev if calling a function with some inputs returns a number, and other times it returns a boolean. Since we're returning a number in the main case, returning 0 for the score for an empty word would be more expected. If we did want to report an error case, we might consider raising an error.

else:
total_score += score_chart[letter]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice lookup of the per-letter score from your dict.

if 7 <= len(word) <= 10:
total_score += 8
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Nice bonus handling.

return total_score

def get_highest_word_score(word_list):
pass

best_words_list = []

for word in word_list:
word = word.upper()
score_one_word = score_word(word)
best_words_list.append((word, score_one_word))
# print(best_words_list)

for word, score in best_words_list:
if len(word) == 10:
return ((word, score))
Comment on lines +94 to +95
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the best_words_list hasn't been filtered by score (and words can appear in any order), we can't say for certain that the first 10 letter word we find is the winner. There might be a higher scoring 10-letter word later in the list.

If we filtered the list so that the only words in it are those with the highest score, then this assumption would be accurate.

else:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The else here isn't needed. Since the loop will either complete without exiting (triggering the else) or return from the function (in which case, there's no risk of running the code following the loop), we know that if we reach the code after the loop, it can only be due to the look exiting normally (there's no break).

As such, we can remove the else and unindent the remaining code.

top_score = 0
best_word = []
for word, score in best_words_list:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Nice tuple unpacking during the iteration.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice way to get the list of words tied for the best score. Consider doing this logic when building the best_words_list (which would address the 10-letter word check).

if score > top_score:
top_score = score
best_word = [word]
elif score == top_score:
best_word.append(word)

short_word = best_word[0]
for word in best_word:
if len(short_word) > len(word):
short_word = word
Comment on lines +107 to +109
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job finding the shortest word from among the words tied for the best score.

From the commented code below, it looks like you were also looking into max/min as wyas to accomplish some of this. Now that you've implemented the logic behind min yourself, I'd definitely suggest incoroporating min/max in future code!

return short_word, top_score






# max_word_score = max(best_words_list, key=lambda x: x[1])
# if len(best_words_list) == 1:
# return best_words_list
# else:
# max_score = max(best_words_list, key=lambda x: x[1])[1]
# max_words = [word for word, score in best_words_list if score == max_score]
# if len(max_words) == 1:
# return max(best_words_list, key=lambda x: x[1])
# else:
# ten_letter_words = [word for word in max_words if len(word) == 10]
# if ten_letter_words:
# return (ten_letter_words[0], max_score)
# else:
# min_length_word = min(max_words, key=lambda x: len(x))
# return (min_length_word, max_score)