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

Tigers - Reyna + Misha #79

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

Tigers - Reyna + Misha #79

Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
065c194
"adds draw_letters(), test_wave_01 passing"
reynamdiaz Sep 28, 2022
d015313
"test_wave_02 in progress, 3/5 tests passing"
reynamdiaz Sep 28, 2022
cd6549f
adds wave 2 functions, passes wave2 tests
misha-joy Sep 28, 2022
25f8a65
adds wave3 function
misha-joy Sep 29, 2022
0e01065
wave 3 again
misha-joy Sep 29, 2022
a5985fa
"wave2 refactor"
reynamdiaz Sep 29, 2022
2886492
wave 2 refactor
reynamdiaz Sep 29, 2022
6f36a10
"wave 2 refactor"
reynamdiaz Sep 29, 2022
1106dd6
"adds wave3 again again"
reynamdiaz Sep 29, 2022
ca93d02
"added wave4 function, all tests passing"
reynamdiaz Sep 30, 2022
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
86 changes: 81 additions & 5 deletions adagrams/game.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,87 @@
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
letters_not_drawn = list(LETTER_POOL.keys())
letters_drawn = []
letter_count = 0

def uses_available_letters(word, letter_bank):
pass
for letter in letters_not_drawn:
Copy link

Choose a reason for hiding this comment

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

I think we could combine the for loop and if statement in one using a while loop. Consider:

while len(letters_draw) < 10:
    # randomly select a letter
    # append to letters_drawn if that letter has any quantity left
    # the rest of your code

But what letter do we draw? Well, it's supposed to be random every time. Instead of running a for loop from start to finish, how can we randomly select a key or an index position? (This may take some Googling)

if letter not in letters_drawn and len(letters_drawn) < 10:
Copy link

Choose a reason for hiding this comment

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

hmm does this mean that the list of letters_drawn will never have duplicate letters? Plenty of words need to use the same letters. That's why the LETTER_POOL pull has values that represent how many of each letter can be pulled.

Instead of checking letter not in letters_drawn, we should check whether or not the letter has a quantity greater than 0 to still draw from.

letters_drawn.append(letter)
letter_count += 1
Copy link

Choose a reason for hiding this comment

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

Doesn't look like we are using this variable anywhere. Let's get rid of it!

return letters_drawn

def uses_available_letters(word, letter_bank):
word = word.upper()
word_dict = {}
is_valid = False
letter_count = 0
for letter in word:
if letter in letter_bank and word.count(letter) <= letter_bank.count(letter):
Comment on lines +23 to +24
Copy link

Choose a reason for hiding this comment

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

we are only just learning about Big O and a function's time and space complexity, but let's explore how we can make this more efficient.

Right now we have a loop inside a loop (if...in has a loop under the hood). That makes the code O(n^2). Consider calculating a frequency map of the letters before entering the for loop on line 23. Building a frequency map is a linear operation aka O(n), but we could look up the letters in constant time aka O(1) with a frequency map (if letter in dictionary versus if letter in list).

That's very technical, but to summarize: using a dictionary to look up each letter in the word and subtracting from the dictionary would be more time efficient than checking if the letter is in the letter_bank list.

word_dict[letter] = True
letter_count += 1
is_valid = True
elif not letter in letter_bank:
word_dict[letter] = False
is_valid = False
return is_valid
Comment on lines +23 to +31
Copy link

Choose a reason for hiding this comment

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

I think we can get rid of the word_dict and letter_count altogether! Both are created and updated, but then they aren't used anywhere to calculate our final result.

Secondly, there's a logic error that the tests didn't pick up on. Let's take the word "ALABAMA" for example. Based on this function:

letter_bank = ["F","A","B","D","A","C","C","L","M","N"]
uses_available_letters("ALABAMA", letter_bank)

it returns True, which it shouldn't. What's causing this? Well, let's take "A" through your conditionals.

if letter in letter_bank and word.count(letter) <= letter_bank.count(letter) 4<= 2 so, no this doesn't execute.

elif not letter in letter_bank "A" is in the letter_bank, so this doesn't execute either.

What can we add to this function to fix this?


def score_word(word):
Copy link

Choose a reason for hiding this comment

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

👍 good job!

pass
score = 0
word = word.upper()
points = {
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"]
}

for letter in word:
for key, value in points.items():
if letter in value:
score += key
if len(word) >= 7:
score += 8
return score

def get_highest_word_score(word_list):
Copy link

Choose a reason for hiding this comment

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

👍

pass
scores_dict = {}
for word in word_list:
scores_dict[word] = score_word(word)
highest_score = max(scores_dict.values())
highest_scoring_words = []
for word, score in scores_dict.items():
if score == highest_score:
highest_scoring_words.append(word)
longest_word = max(highest_scoring_words, key=len)
shortest_word = min(highest_scoring_words, key=len)
Comment on lines +63 to +64
Copy link

Choose a reason for hiding this comment

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

👍 nice job using max and min so effectively!

winning_word = []
if len(longest_word) >= 10:
winning_word.append(longest_word)
winning_word.append(score_word(longest_word))
else:
winning_word.append(shortest_word)
winning_word.append(score_word(shortest_word))
return tuple(winning_word)