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

Spruce - Zandra & Reid #44

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

# Wave 1


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
"""
Uses dict to create a list of the letter pool.
Shuffles list and returns slice of first 10 items.
"""
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
}
Comment on lines +11 to +38
Copy link
Collaborator

Choose a reason for hiding this comment

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

Long chunks of data like letter_bank tend to clutter functions and require more scrolling to see the rest of the function body. Consider moving this dictionary outside of this function (or in a new file) as a constant variable to be referenced in draw_letters.


letter_list = []
for letter in LETTER_POOL:
counter = 0
while counter < LETTER_POOL[letter]:
letter_list.append(letter)
counter += 1

random.shuffle(letter_list)

return letter_list[0:10]
Comment on lines +47 to +49
Copy link
Collaborator

Choose a reason for hiding this comment

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

Really cool combo of the shuffle method and slicing to generate a random hand of letters!


# Wave 2


def uses_available_letters(word, letter_bank):
pass
"""
Makes copy of letter bank list.
Checks to see if all letters in the words are in the letter bank.
"""

letter_bank_copy = []
for item in letter_bank:
letter_bank_copy.append(item)
Comment on lines +60 to +62
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 eliminate the for loop and reduce these 3 lines into 1 by using the copy method to generate a copied list from letter_bank.

Suggested change
letter_bank_copy = []
for item in letter_bank:
letter_bank_copy.append(item)
letter_bank_copy = letter_bank.copy()


for char in word:
if char in letter_bank_copy:
letter_bank_copy.remove(char)
else:
return False
Comment on lines +64 to +68
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 char not in letter_bank_copy is the 'guard' that checks if char does not meet the criteria to make the loop continue. In this case, if char is not in the 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 char in word:
if char in letter_bank_copy:
letter_bank_copy.remove(char)
else:
return False
for char in word:
if char not in letter_bank_copy:
return False
letter_bank_copy.remove(char)


return True

# Wave 3


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.

👍 Nice job! The dictionary makes this solution neat. To make this function even neater, try moving score_chart outside the function as a constant variable.

pass
"""
Iterates through all letters in word, adds corresponding value to sum.
Adds 8 at the end if length of word greater than 6.
"""

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

word = word.upper()
score = 0
for letter in word:
score += score_chart[letter]

if len(word) > 6:
score += 8

return score

# Wave 4


def get_highest_word_score(word_list):
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 The logic for this function looks great! It's really easy to get bogged down into unnecessary nested conditionals for this function so great work in keeping the conditionals very neat.

pass
"""
Iterates through word list and calculates each score.
Checks score, length and returns winning word and score.
"""
word_result = ""
score_count = 0
for given_word in word_list:
each_word_score = score_word(given_word)
if each_word_score > score_count:
score_count = each_word_score
word_result = given_word
elif each_word_score == score_count:
if len(given_word) != len(word_result) and len(given_word) == 10:
word_result = given_word
elif len(given_word) < len(word_result) and len(word_result) != 10:
word_result = given_word

return [word_result, score_count]
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
return [word_result, score_count]
return word_result, score_count

or

Suggested change
return [word_result, score_count]
return (word_result, score_count)