Skip to content

Kunzite - Allie S. #122

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 10 commits into
base: main
Choose a base branch
from
130 changes: 126 additions & 4 deletions adagrams/game.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,133 @@
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
letter_list = []

# While the length of letter_list list is not 10, continue finding an available letter
while len(letter_list) != 10:

Choose a reason for hiding this comment

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

Consider using while len(letter_list) < 10. This condition casts a wider net of catching issues than only stopping when len(letter_list) reaches 10. A bug could be introduced upon refactoring, and then jump past 10 into 11 or 12.

# convert the keys in the LETTER_POOL dictionary into a list
# randomly choose a letter form this list
letter = random.choice(list(LETTER_POOL.keys()))

Choose a reason for hiding this comment

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

Nice idea! One thing to consider: should each letter have the same probability of being chosen, though? For example, should "Z" have a probability of 1 in 26 or 1 in 98 (the total amount of tiles that can be drawn)?

How could we represent that in a data structure? Maybe a list of all tiles and their duplicates (ie, ['A','A','A','B','B'] and so on) to randomly choose from.

A more advanced solution might be using the random.choice method, but with more than one param. Here's an example of how you can "weigh" your string or list: https://www.geeksforgeeks.org/how-to-get-weighted-random-choice-in-python/

# check if the random letter selected is available in the LETTER_POOL dicrionary
if LETTER_POOL[letter] > 0:
# if the letter is available, append the selected letter to the letter_list list
letter_list.append(letter)
# once the letter is used, decrement the quantity available for this letter in the LETTER_POOL dictionary
LETTER_POOL[letter] -= 1
# return letter_list list that contains 10 randomly selected letters
return letter_list


def uses_available_letters(word, letter_bank):
pass
# makes a list copy of the letter_bank list so as not to modify the original list
letter_bank_copy = letter_bank.copy()
# iterates through each capitalized letter in the word the user passes in
for letter in word.upper():
# checks if the current letter if found in the letter_bank_copy list
if letter in letter_bank_copy:
# if the current letter is found, remove that instance from the list
letter_bank_copy.remove(letter)
Comment on lines +55 to +59

Choose a reason for hiding this comment

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

This works fine! We've only just talked about Big O and time/space complexity, but one thing to notice here is we have a 3 nested for loops, even if we haven't explicitly stated for___ in ___ three times.

The remove method loops through letter_bank_copy and if letter in letter_bank_copy loops under the hood. Nothing wrong with 2 nested for loops. At times they are the easier solution. But! we have 3 here, and a dictionary of letters counted from the letter_bank would help us here.

Same with remove. It must find the letter, remove it, then shift any values over to fill the space.

How might we create a "frequency map" (a dictionary of items counted) with a constant lookup (O(1)) of letters found in letter_bank, then use it to check if all those letters are in the word?

else:
# on the first instance the current letter is not found in the letter_bank_copy list, return False
return False
# ends function and returns True if all of the letters in word are found in the letter_bank list
return True


def score_word(word):
pass
# SCORE_CHART is a chart that contains the point value for each letter
SCORE_CHART = {
"AEIOULNRST": 1,
"DG": 2,
"BCMP": 3,
"FHVWY": 4,
"K": 5,
"JX": 8,
"QZ": 10
}
# initializes score variable that will keep track of the accumulating score for each letter found in the user's word
score = 0
# grabs the key and value of each dicionary key-pair in the SCORE_CHART dictionary
for key, value in SCORE_CHART.items():
# iterate through each letter in capitalized word
for letter in word.upper():
# searches for each letter in a key of letters from SCORE_CHART
if letter in key:
Comment on lines +81 to +85

Choose a reason for hiding this comment

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

Here again we've got 3 nested loops. Let's refactor the SCORE_CHART dictionary so that each letter is its own key. Is there some overhead setting that up? Yes. But it would also let us look up a letter's score in "constant" time (letter in score_dict) rather than iterating through an entire string, which is "linear" time (letter in "AEIOULNRST").

Again, we only just discussed time complexity and Big O, but that's a brief explanation as to why a dictionary is a beneficial data structure we can use to hold data that needs to be frequently looked up.

# if the current letter is found in any of the keys from SCORE_CHART, add that key's value to the score
score += value
# checks if the length of the word is between 7 and 10
if 7 <= len(word) <= 10:
# if the length of the word is between 7 and 10, an additional 8 bonus points are added to score
score += 8
# returns the final score
return score


def get_highest_word_score(word_list):
pass
# initialize best_word variable that will hold the best_word
best_word = ""
# initialize highest_score variable that will hold the highest_score
highest_score = 0
# iterate through numvers 0 to the length of the word_list
for i in range(0, len(word_list)):
# grab the score for the current word iteration by calling score_word function
current_word_score = score_word(word_list[i])
# checks if the current_word_score iteration is greater than the highest_score
if current_word_score > highest_score:
# if current_word_score is greater than highest_score, update best_word and highest_score
best_word = word_list[i]
highest_score = current_word_score
# else if current_word_scre equals highest_score:
elif current_word_score == highest_score:
# if current_word_score is equal to highest_score, keep first instance
if (len(word_list[i]) == 10) and (len(best_word) == 10):
best_word = best_word
highest_score = highest_score
Comment on lines +113 to +115

Choose a reason for hiding this comment

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

Hm, do we need this conditional? We are re-assigning the current highest scored word with itself. Let's get rid of it.

# else if the length of current word_list iteration is 10 while the length of best_word is not equal to 10, then update best_word and highest_score
elif (len(word_list[i]) == 10) and (len(best_word) != 10):
best_word = word_list[i]
highest_score = current_word_score
# else if the length of current word_list iteration is not 10 while the length of best_word is equal to 10, keep current best_word and highest_score
elif (len(word_list[i]) != 10) and (len(best_word) == 10):
best_word = best_word
highest_score = highest_score
# else if the length of current word_list iteration is greater than the length of best_word, keep current best_word and highest_score
elif len(word_list[i]) > len(best_word):
best_word = best_word
highest_score = highest_score
Comment on lines +125 to +127

Choose a reason for hiding this comment

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

Again, we can get rid of this conditional altogether because the current highest word is already assigned to the best_word variable.

# else if the length of current word_list iteration is less than the length of best_word, update best_word and highest_score
elif len(word_list[i]) < len(best_word):
best_word = word_list[i]
highest_score = current_word_score
# return the final results of best_word and highest_score
return best_word, highest_score
58 changes: 31 additions & 27 deletions tests/test_wave_01.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,41 +3,43 @@
from adagrams.game import draw_letters

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,
'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 test_draw_letters_draws_ten():
# Arrange/Act
letters = draw_letters()

# Assert
assert len(letters) == 10


def test_draw_letters_is_list_of_letter_strings():
# Arrange/Act
letters = draw_letters()
Expand All @@ -49,6 +51,7 @@ def test_draw_letters_is_list_of_letter_strings():
assert type(elem) == str
assert len(elem) == 1


def test_letter_not_selected_too_many_times():

for i in range(1000):
Expand All @@ -61,16 +64,17 @@ def test_letter_not_selected_too_many_times():
letter_freq[letter] += 1
else:
letter_freq[letter] = 1

# Assert
for letter in letters:
assert letter_freq[letter] <= LETTER_POOL[letter]


def test_draw_letters_returns_different_hands():
# Arrange/Act
hand1 = draw_letters()
hand2 = draw_letters()
hand3 = draw_letters()

# Assert
assert hand1 != hand2 or hand2 != hand3
assert hand1 != hand2 or hand2 != hand3