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.

Sign up for GitHub

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- Asli A. and Ivette F. #51

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Spruce- Asli A. and Ivette F. #51

Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3a5cd53
finished wave 1 with passing tests
IvetteDF Sep 22, 2021
041a0a8
implemented uses_available_letters() finish wave2 and passes wave2 tests
IvetteDF Sep 23, 2021
09d2884
finished wave 3, passed all tests
asliathman Sep 24, 2021
f4ea0fd
fixed formating, moved score_chart dictionary to be a global variable
asliathman Sep 25, 2021
e69e3ef
started working on wave 4, tests not passing
asliathman Sep 25, 2021
85f623e
no updates
IvetteDF Sep 25, 2021
26912ae
no updates
IvetteDF Sep 25, 2021
d8ce9ab
no updates
IvetteDF Sep 25, 2021
0c5253b
copying get_highest_word() from previous commit trying to fix what I …
IvetteDF Sep 25, 2021
b491a6a
all waves passing tests and updated wave4
IvetteDF Sep 25, 2021
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
110 changes: 106 additions & 4 deletions adagrams/game.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,113 @@
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
}

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 constant variables, especially outside of functions. These variables could live inside of functions but they definitely add more clutter to the function body than necessary.

Having constant variables or any other data containing large chunks allows the function body to focus on the important logic (aka the purpose of the function).

SCORE_CHART = {
1: ['a', 'e', 'i', 'o', 'u', 'l', 'n', 'r', 's', 't'],
2: ['d', 'g'],
3: ['b', 'c', 'm', 'p'],
4: ['f', 'h', 'v', 'w', 'y'],
5: ['k'],
8: ['j', 'x'],
10: ['q', 'z'],
}
Comment on lines +32 to +40
Copy link
Collaborator

Choose a reason for hiding this comment

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

Love how y'all used the score as the key in this dictionary, very clever!



def draw_letters():
pass
letter_hand = []
letter_pool_list = []
for letter, number in LETTER_POOL.items():
letter_pool_list.extend([letter] * number)
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 extend to create a list of letters according to their letter count in the LETTER_POOL. You can omit the brackets around letter as well and it will create that same single-dimension list. Was the intention to create a nested list?

Suggested change
letter_pool_list.extend([letter] * number)
letter_pool_list.extend(letter * number)

for i in range(10):
letter = random.choice(letter_pool_list)
letter_hand.append(letter)
letter_pool_list.remove(letter)
Comment on lines +48 to +51
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good work in adding the remove method to ensure that the letters being picked are valid!

return letter_hand


def uses_available_letters(word, letter_bank):
pass
word_checker = letter_bank.copy()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good job making a copy to avoid the side-effects of altering the letter bank!

for char in word:
if char not in word_checker:
return False
else:
word_checker.remove(char)
return True


def score_word(word):
pass
word = word.lower()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nitpick/Opinion/🧅: If you can keep letter cases and data types consistent throughout a program then absolutely try to! Storing capital letters in SCORE_CHART would have eliminated the need for this line.

However, this is good forecasting for working with APIs/external sources of data. Oftentimes, the data we retrieve from other sources are not in the data structure we expect resulting in transforming the data into the correct letter case or data type for our program (like y'all did here).

num_points = 0
for char in word:
for score, letters in SCORE_CHART.items():
if char in letters:
num_points += score
if len(word) >= 7:
num_points += 8
return num_points
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function is so clean due to how the points and letters were stored in SCORE_CHART. Great work!!



def get_highest_word_score(word_list):
pass
#returns tuple with the highest scored word, and the score
#find the scores for each word in word list
#find the highest score

words_and_scores = []

for word in word_list:
score = score_word(word)
words_and_scores.append((word, score))
Comment on lines +84 to +86
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 score_word function and storing tuples in a list!


highest_score = 0
for pair in words_and_scores:
if pair[1] > highest_score:
highest_score = pair[1]

high_score_words = []
for pair in words_and_scores:
if pair[1] == highest_score:
high_score_words.append(pair)
Comment on lines +88 to +96
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of making two separate loops to iterate through the same collection, we can combine the logic into one! Doing so will reduce the number of total iterations the algorithm would execute, resulting in a (slightly) faster algorithm.

Generally, if we have to iterate through the same collection multiple times, we might as well try to find a way to combine the logic (in this case conditionals) into a single loop. Kind of like grabbing all the groceries from your car at once rather than going back and doing multiple trips 🤣 , it saves you time!

This will require one additional line of reassigning high_score_words into creating a list with a new pair containing the highest score.

Suggested change
highest_score = 0
for pair in words_and_scores:
if pair[1] > highest_score:
highest_score = pair[1]
high_score_words = []
for pair in words_and_scores:
if pair[1] == highest_score:
high_score_words.append(pair)
highest_score = 0
high_score_words = []
for pair in words_and_scores:
if pair[1] > highest_score:
highest_score = pair[1]
high_score_words = [pair]
elif pair[1] == highest_score:
high_score_words.append(pair)


# tie breaking logic
if len(high_score_words) > 1:
for pair in high_score_words:
if len(pair[0]) == 10:
return pair

shortest_length = 10
for pair in high_score_words:
if len(pair[0]) < shortest_length:
shortest_length = len(pair[0])

for pair in high_score_words:
if len(pair[0]) == shortest_length:
return pair
Comment on lines +100 to +111
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 reduce this section into one loop as well. However, this would require the pair with the shortest word length to be stored and later reassigned if the next iteration contained a shorter word.

Suggested change
for pair in high_score_words:
if len(pair[0]) == 10:
return pair
shortest_length = 10
for pair in high_score_words:
if len(pair[0]) < shortest_length:
shortest_length = len(pair[0])
for pair in high_score_words:
if len(pair[0]) == shortest_length:
return pair
shortest_length = 10
shortest_pair = ()
for pair in high_score_words:
if len(pair[0]) == 10:
return pair
elif len(pair[0]) < shortest_length:
shortest_length = len(pair[0])
shortest_pair = pair
elif len(pair[0]) == shortest_length:
return pair
return shortest_pair

else:
return high_score_words[0]