Skip to content

Tigers - Mica C. and Lain #64

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

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
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
86 changes: 82 additions & 4 deletions adagrams/game.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,89 @@
LETTER_POOL = {
"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,
}

LETTER_POINTS = {
"A": 1,
"N": 1,
"B": 3,
"O": 1,
"C": 3,
"P": 3,
"D": 2,
"Q": 10,
"E": 1,
"R": 1,
"F": 4,
"S": 1,
"G": 2,
"T": 1,
"H": 4,
"U": 1,
"I": 1,
"V": 4,
"J": 8,
"W": 4,
"K": 5,
"X": 8,
"L": 1,
"Y": 4,
"M": 3,
"Z": 10,
}


def draw_letters():

Choose a reason for hiding this comment

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

This looks fine, good use of library functions and built-in operators! No comments.

pass
from random import sample

pool = "".join(letter * count for letter, count in LETTER_POOL.items())
return sample(pool, k=10)


def uses_available_letters(word, letter_bank):

Choose a reason for hiding this comment

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

This is a fine implementation very succinct and pythonic, though there's a slight time performance hit from using count...

pass
upper_word = word.upper()
return all(
upper_word.count(letter) <= letter_bank.count(letter)
for letter in set(upper_word)

Choose a reason for hiding this comment

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

From my investigations, count takes O(n) time as the implementation goes through each element of the list and just counts up. The for generator runs over set(upper_word), which at worst has n elements (n being the number of letters in word) if every letter in word is different. Thus O(n^2) in the end for time complexity.

A common way to implement this function is to create frequency dictionaries of either or both word and letter_bank (each likely taking a separate O(n) for loop) and then compare the counts, which could also be done in an O(n) loop. Since none of these loops are nested, the entire implementation takes O(n).

However, by creating the dictionaries, we are also using O(n) space, while the big benefit of generators is that they don't need to allocate space, making y'all's implementation O(1). Which could very well be worth the worse time complexity! Trade-offs abound.

)


def score_word(word):

Choose a reason for hiding this comment

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

This is also a very fine succinct and pythonic implementation, but I think then we get a space performance issue this time...

pass
total_score = sum([LETTER_POINTS.get(letter, 0) for letter in word.upper()])

Choose a reason for hiding this comment

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

In this case rather than a generator, y'all used a comprehension (due to the square brackets), which does end up allocating O(n) memory (apparently even if you never assign it, sadly). However, I just tested and y'all could just switch to a generator here and get O(1) space complexity!

Great since this is so succinct and clean and I do really value that!

total_score += len(word) > 6 and 8

Choose a reason for hiding this comment

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

Good use of the short-circuiting behavior of and as well as the integer representation of False as 0. I could quibble about readability, but these behaviors are common enough knowledge it's not that big a deal to me.

return total_score


def break_tie(words):

Choose a reason for hiding this comment

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

Woo, helper functions!

return min(words, key=lambda w: 0 if len(w) == 10 else len(w))

Choose a reason for hiding this comment

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

And this is so good and succinct to account for all tiebreakers and verifying that min (and its siblings) pick the first encountered matching value it does indeed meet all the tie breaking rules.

Nice work!



def get_highest_word_score(word_list):

Choose a reason for hiding this comment

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

Nice and succinct as all the rest of the code has been while still being correct. No comments.

pass
word_scores = {word: score_word(word) for word in word_list}
highest_score = max(word_scores.values())
tied_words = [word for word, score in word_scores.items() if score == highest_score]
return break_tie(tied_words), highest_score