Skip to content

Cheetah C18 Viktoriia Zolotova & Vivian Zhu #62

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 12 commits into
base: master
Choose a base branch
from
171 changes: 165 additions & 6 deletions adagrams/game.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,170 @@
def draw_letters():
pass
from operator import length_hint
from platform import win32_edition
import random
from lib2to3.pgen2.pgen import DFAState

# initiate the empty list
# to keep the original pool - make a copy of dictionary
# get letter from the copy dict/pool random.ran()
# draw one letter a time, x 10, for loop (0, 10),
# also update the qty. of the letter in each loop
LETTER_POOL = {

Choose a reason for hiding this comment

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

Nice job creating constants for the letter pool and letter scores! ✨

'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_POOL = {
'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
}

def copy_dictionary(pool):

Choose a reason for hiding this comment

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

Nice helper function! I'd also recommend checking out the deepcopy() function. :)

"""
Returns a copy of a dictionary.
"""
pool_dict_copy = {}
for key, value in pool.items():
pool_dict_copy[key] = value
return pool_dict_copy

def draw_letters():
"""
Returns an array of ten random strings from letter pool.
"""
res_letters = []
letter_bank_dict = copy_dictionary(LETTER_POOL)
while len(res_letters) < 10:
letter = random.choice(list(letter_bank_dict.keys()))
if letter_bank_dict[letter] > 0:

Choose a reason for hiding this comment

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

This approach to drawing ten random letters absolutely works, but consider a scenario where the random.choice() function keeps selecting letters that are already at a frequency of 0. There's no telling how many times this loop may need to run! A small change we can make to guarantee our loop will only run 10 times is removing the letter from the dictionary once its frequency is at 0.

To accomplish this, we could refactor your loop to look something like this:

while len(res_letters) < 10:
    letter = random.choice(list(letter_bank_dict.keys()))
    res_letters.append(letter)
    letter_bank_dict[letter] -= 1
    if letter_bank_dict[letter] == 0:
        del letter_bank_dict[letter]

res_letters.append(letter)
letter_bank_dict[letter] -= 1
return res_letters

def create_dict(data):
"""
Returns dictionary with key "element" and
with value equal to the frequency of the element.
"""
data_dict = {}
for element in data:
if element not in data_dict:

Choose a reason for hiding this comment

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

I'd recommend checking out the dictionary setdefault() method. This method could help clean up this if-else block by handling situations where the dictionary key doesn't exist. ☺️

data_dict[element] = 1
else:
data_dict[element] += 1
return data_dict

def uses_available_letters(word, letter_bank):
pass
"""
Returns True if every letter from word
belongs to the letter bank, otherwise False.
"""
word = word.upper()
word_dict = create_dict(word)
letter_bank_dict = create_dict(letter_bank)

for letter, quantity in word_dict.items():

Choose a reason for hiding this comment

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

Nice job with your uses_available_letters() function! ✨

(optional) There is a solution for this function that only uses one loop and doesn't require any additional data structures to be created. See if you can optimize your space and time complexity! 🤔😌

if letter not in letter_bank_dict:
return False
else:
if quantity > letter_bank_dict[letter]:
return False
return True

def score_word(word):
pass
def score_word(word):
"""
Returns the score of the given word.
"""
total_score = 0
word = word.upper()
if len(word) >= 7 and len(word) <= 10:
total_score += 8
for letter in word:
if letter in SCORE_POOL.keys():
total_score += SCORE_POOL[letter]
return total_score

Choose a reason for hiding this comment

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

Nice job with your score_word() function! ✨


def get_highest_word_score(word_list):
pass
"""
Returns a tuple of a winning word and it's score.
"""
max_score = 0
win_name = ""
tie_word_dict = {}
for word in word_list:
word_score = score_word(word)
if word_score > max_score:
max_score = word_score
win_name = word

for word in word_list:

Choose a reason for hiding this comment

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

This loop header and the loop header on line 141 are identical. Consider how you can combine these two loops to prevent having to iterate over the same data twice.

if score_word(word) == max_score:
tie_word_dict[word] = len(word)

# check if only one word in tie_word_dict
if len(tie_word_dict) == 1:
return win_name, max_score

#check if len of word in tie_word_dict = 10, this word win
for word, length in tie_word_dict.items():
while length == 10:

Choose a reason for hiding this comment

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

Not sure if this was a typo, but this line should be an if statement rather than a while loop. It still works because even though it's a loop, there is a return statement inside. So, when we find a word that is of length 10, we enter the loop and then immediately exit the loop and the function.

return (word, max_score)

#check if len of word != 10, word with fewest len win
#and if words with the same length -> pick the first one in tie_word_dict
else:

Choose a reason for hiding this comment

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

Interesting that you have implemented a for-else loop here! This structure works here, but for-else loops are really meant to be used when we're searching for values with loops. They're great for conditionally running code if a value wasn't found. You can read more about them here, but this is a more advanced topic and they really aren't used very often. So, no pressure to learn about them.

For this function, I would recommend removing the else statement and having the rest of the code directly after the for loop.

mini_length = 10
for word, length in tie_word_dict.items():
if length < mini_length:
mini_length = length
key_list = list(tie_word_dict.keys())
val_list = list(tie_word_dict.values())
win_name = key_list[val_list.index(mini_length)]

Choose a reason for hiding this comment

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

This is an interesting way to find the word matching the mini_length! Be careful though because this does require creating two new data structures in memory (key_list and val_list) and the .index() method is an O(n) operation. Consider how you could track the word with the shortest length alongside mini_length in the for loop on line 164.

return (win_name, max_score)
1 change: 1 addition & 0 deletions tests/test_wave_01.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,4 @@ def test_letter_not_selected_too_many_times():
# Assert
for letter in letters:
assert letter_freq[letter] <= LETTER_POOL[letter]

3 changes: 2 additions & 1 deletion tests/test_wave_02.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,5 @@ def test_uses_available_letters_ignores_case():
assert uses_available_letters("bEd", letters)
assert uses_available_letters("fad", letters)
assert uses_available_letters("a", letters)
assert not uses_available_letters("aA", letters)
assert not uses_available_letters("aA", letters)

4 changes: 3 additions & 1 deletion tests/test_wave_03.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from adagrams.game import score_word


def test_score_word_accurate():
# Assert
assert score_word("A") == 1
Expand All @@ -23,4 +24,5 @@ def test_score_extra_points_for_seven_or_longer():
assert score_word("XXXXXXX") == 64
assert score_word("XXXXXXXX") == 72
assert score_word("XXXXXXXXX") == 80



4 changes: 3 additions & 1 deletion tests/test_wave_04.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from adagrams.game import score_word, get_highest_word_score


def test_get_highest_word_score_accurate():
# Arrange
words = ["X", "XX", "XXX", "XXXX"]
Expand Down Expand Up @@ -84,4 +85,5 @@ def test_get_highest_word_tie_same_length_prefers_first():
assert score_word(words[0]) == 18
assert score_word(words[1]) == 18
assert best_word[0] == words[0]
assert best_word[1] == 18
assert best_word[1] == 18