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

Zoisite - Gweneth #117

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Zoisite - Gweneth #117

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2b3af08
message
Quizsnak Mar 22, 2023
3602149
FINISHED
Quizsnak Mar 23, 2023
8ee27cc
turning in
Quizsnak Mar 24, 2023
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
141 changes: 137 additions & 4 deletions adagrams/game.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,144 @@
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
}


def draw_letters():
pass
tiles = [] #list that's ten letters
count = 0 #going to stop at 10
Comment on lines +33 to +34
Copy link

Choose a reason for hiding this comment

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

Consider renaming the variable tiles to something like list_of_ten_tiles and then you won't need to add a comment to explain what the variable is for.

If you rename count to something like length_of_hand then when I read the while loop on 36, I can understand that the number of tiles in a hand shouldn't exceed ten. Count is a little vague here, renaming the variable would enable you to get rid of the comment too on line 34 too.

letters_dict = LETTER_POOL.copy()
while count != 10:
Copy link

Choose a reason for hiding this comment

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

What if we accidentally introduced a bug and somehow count became 11? Would the loop still run?

Could we make the condition here a little more specific? Like while count < 10:

letter = random.choice(list(letters_dict.keys()))
Copy link

Choose a reason for hiding this comment

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

list(letters_dict.keys()) returns a list with 26 elements which are the letters A-Z. This doesn't quite represent the distribution of letters we should be selecting a random tile from. Instead, random.choice() should be passed in a list that has 9 As, 2 Bs, 3 Cs, etc.

How would you create a list of letters that represents all the available tiles to draw from by using letters_dict? Could you iterate through the key value pairs of the dictionary and append to a list like all_available_tiles to create this pool to draw from?

if letters_dict[letter] > 0:
letters_dict[letter] -= 1
tiles.append(letter)
count += 1
else:
continue
Comment on lines +42 to +43
Copy link

Choose a reason for hiding this comment

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

Do we need this else statement here since we only ask it to continue?

On line 38, if a the value of a letter is greater than 0, then we do some logic on lines 39-41. If if letters_dict[letter] > 0: is false, then it won't enter that block of code and nothing will execute and the loop starts over again so we can drop lines 42-43 altogether.

return tiles


def uses_available_letters(word, letter_bank):
pass
letter_bank_dict = {}
word_dict = {}
word = word.upper()
Copy link

Choose a reason for hiding this comment

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

👍


for letter in letter_bank:
if letter not in letter_bank_dict:
letter_bank_dict[letter] = 1
else:
letter_bank_dict[letter] += 1
Comment on lines +52 to +56
Copy link

Choose a reason for hiding this comment

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

Nice job building up your frequency dictionary for your available letters! Another approach is to use the dictionary method .get() because it allows you to pass the function a default value to the method.

for letter in letter_bank:
    letter_bank_dict[letter] = letter_bank_dict.get(letter, 0) + 1

So we loop over letter_bank with a for loop, then we set the key of the dict to letter_bank_dict[letter] and the value is what letter_bank_dict.get(letter, 0) + 1 evaluates to. This is a pretty common pattern for building up a frequency dictionary so I wanted to call it out so you're familiar with it.


for character in word:
Copy link

Choose a reason for hiding this comment

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

You can use the variable name letter in this for loop too instead of character if you'd like. While this for loop shares the same local scope as the for loop on line 52, the first loop finished running and using letter again works too.

if character not in letter_bank:
return False
else:
if character not in word_dict:
word_dict[character] = 1
else:
word_dict[character] += 1

for characters in word_dict:
if word_dict[characters] > letter_bank_dict[characters]:
return False
Comment on lines +67 to +69
Copy link

Choose a reason for hiding this comment

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

I see that you create two dictionaries letter_bank_dict which is a frequency of how many times the letters in the list letter_bank appear and you also have word_dict which tells us how many times a letter in word appears.

Do we need to create two separate dictionaries in order to see if word uses the available letters in letter_bank? I think we can get away without needing to create several data structures.

While this isn't a huge issue, it does add complexity because other coders need to figure out why we have several data structures and how they work together. Also, we haven't covered this yet but in our Big O lesson, we'll see how creating data structures whose size grows in relation to a function's input can effect performance.

What if we took a different approach? You could still leverage letter_bank_dict and use this dictionary to check that the letters in word are available.

    for letter in letter_bank:
        if letter not in letter_bank_dict:
            letter_bank_dict[letter] = 1
        else:
            letter_bank_dict[letter] += 1

    for letter in word:
        if letter not in letter_bank:
            return False
        else:
            if letter_bank_dict[letter] > 0:
                letter_bank_dict[letter] = letter_bank_dict[letter] - 1
            else: 
                return False
    
    return True

return True

def score_word(word):
pass
point_values = {
Copy link

Choose a reason for hiding this comment

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

Since point_values is a constant variable that we don't intend to change, we can convey that by naming it POINT_VALUES

"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,
}
total_points = 0
word = word.upper()

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

for letter in word:
letter_point_amount = point_values[letter]
total_points += letter_point_amount
Comment on lines +108 to +109
Copy link

Choose a reason for hiding this comment

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

We can make this a little more concise by directly adding the value from point_values[letter] to total_points. So you would remove line 108 and line 109 would say:
total_points += point_values[letter]

Using descriptive variable names can be helpful, but for simple logic like this we can also get away without it to keep our code concise.

There's no hard and fast rule and opinions may differ about this too.

return total_points


def get_highest_word_score(word_list):
pass
words_points = {}
for word in word_list:
points = score_word(word)
if points not in words_points.keys():
Copy link

Choose a reason for hiding this comment

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

If you only need to iterate over the keys, we don't need .keys(). We can find if a key is in a dict just with if key not in dict

words_points[points] = [word]
else:
words_points[points].append(word)

highest_key = max(words_points.keys())
Copy link

Choose a reason for hiding this comment

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

Same comment as above with .keys(), we can find the max value of the keys in a dict just with max(words_points)

It is nice to be able to use a built in function like max instead of writing our own logic out. It is more concise overall. How do you understand the max() function? How does it work under the hood?

You'll find me asking you about built in functions to ensure that you're clear on what's happening under the hood in case it comes up in an interview.

highest_words_list = words_points[highest_key]

current_winner = highest_words_list[0]
for index in range(1, len(highest_words_list)):
if len(current_winner) == 10:
return (current_winner, highest_key)
elif len(highest_words_list[index]) == 10:
return (highest_words_list[index], highest_key)
elif len(current_winner) > len(highest_words_list[index]):
current_winner = highest_words_list[index]
return (current_winner, highest_key)
Comment on lines +126 to +133
Copy link

Choose a reason for hiding this comment

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

Nice solution here! I see what you're doing by starting the range at 1 which enables us to skip over the current_winner and compare other subsequent tied words if necessary. It is a little unclear to read at first glance and I fired up your code with the debugger to see what was going on.

I wonder if this for loop could be reworked with a for-in loop instead to clarify what's going on.











Comment on lines +134 to +143
Copy link

Choose a reason for hiding this comment

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

Delete extra white space to keep your file neat and concise

draw_letters()
Copy link

Choose a reason for hiding this comment

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

Looks like you added a function call to the end of this file. Be sure to delete unnecessary function calls when you're done working on your project.

While not a big deal here and the tests still pass, imagine a scenario where this method actually calls a database to get the letters for the hand. That call to the database to get the data may take a long time so accidentally leaving in a function call like this could increase latency time for production users so we should be careful about this.