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.

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

C16 - Spruce - Sandra Caballero and Vange Spracklin #46

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
196 changes: 192 additions & 4 deletions adagrams/game.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,199 @@
import random

LETTER_POOL = (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Great choice in using a tuple of nested dictionaries. While we encourage copying any data structure rather than altering it directly, the tuple adds an extra layer of protection.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I also like how the tuple is stored as a constant variable to prevent clogging up other functions. Keeping the functions nice and focused!

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

def draw_letters():
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

pass
hand_of_letters = []
letter_pool_list = []

for letter_dict in LETTER_POOL:
for key in letter_dict:
freq = letter_dict[key]["frequency"]
for i in range(0, freq):
letter_pool_list.append(key)

for i in range(10):
index = random.randint(0, len(letter_pool_list)-1)
letter = letter_pool_list[index]
hand_of_letters.append(letter)
letter_pool_list.pop(index)
Comment on lines +146 to +150
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good use of pop to ensure that a letter isn't selected more than it's frequency.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Optionally, random.sample() can be used here to generate the random letters instead of this for loop.

Suggested change
for i in range(10):
index = random.randint(0, len(letter_pool_list)-1)
letter = letter_pool_list[index]
hand_of_letters.append(letter)
letter_pool_list.pop(index)
hand_of_letters = random.sample(letter_pool_list, 10)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Here's more info on the sample in case it will come handy in the future: https://docs.python.org/3/library/random.html#random.sample


return hand_of_letters

def uses_available_letters(word, letter_bank):
pass
letter_bank_copy = letter_bank[:]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Great use of the slice operator to make a shallow copy.

word_character_list = list(word)

for letter in word_character_list:
if letter in letter_bank_copy:
letter_bank_copy.remove(letter)
else:
return False
return True
Comment on lines +158 to +163
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can restructure this block by reversing the conditional logic into a guard clause. The if letter not in letter_bank_copy will become the 'guard' that checks if letter does not meet the criteria to make the loop continue. In this case, if letter is not in letter_bank_copy list then the loop will exit early and return False.

As Ansel describes, the guard clause removes the need for dangly bits like the else clause.

Suggested change
for letter in word_character_list:
if letter in letter_bank_copy:
letter_bank_copy.remove(letter)
else:
return False
return True
for letter in word_character_list:
if letter not in letter_bank_copy:
return False
letter_bank_copy.remove(letter)
return True


def score_word(word):
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

pass
total_score = 0

word = word.upper()

for letter in word:
for letter_dict in LETTER_POOL:
for letter_key, letter_data in letter_dict.items():
if letter_key == letter:
total_score += letter_data["point_value"]

if len(word) >= 7:
total_score += 8

return total_score

def get_highest_word_score(word_list):
pass
top_word = ""
top_score = 0

for word in word_list:
if score_word(word) > top_score:
top_word = word
top_score = score_word(word)
elif score_word(word) == top_score:
if len(word) == 10 and len(top_word) == 10:
continue
elif len(top_word) == 10:
continue
Comment on lines +190 to +193
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can combine these two into a combined conditional expressions since they contain the same logic:

Suggested change
if len(word) == 10 and len(top_word) == 10:
continue
elif len(top_word) == 10:
continue
if len(word) == 10 and len(top_word) == 10 or len(top_word) == 10:
continue

elif len(word) == 10 or len(word) < len(top_word):
top_word = word
top_score = score_word(word)

best_word = [top_word, top_score]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Our tests don't check for the return type, but the word and the score should be returned as a tuple rather than a list. Tuples, like lists, also use indexing to access values which is why your solution passed the tests.

Suggested change
best_word = [top_word, top_score]
return word_result, score_count

or

Suggested change
best_word = [top_word, top_score]
best_word = (top_word, top_score)

return best_word