Skip to content

Phoenix class - Aksana #34

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
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
156 changes: 151 additions & 5 deletions adagrams/game.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,157 @@
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
}

score_dict = {

Choose a reason for hiding this comment

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

Just a small thing, but remember that the naming convention for a global variable would be all caps! (SCORE_DICT = )

'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
}


def draw_letters():
pass
# Create a list of letters using their frequencies
# pull letters from the list using random.randint
# create a new list from 10 random letters
list_letters = []

for letter, freq in LETTER_POOL.items():

Choose a reason for hiding this comment

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

Great job creating a list to account for the different letter frequencies! In the future, if you want to make something like this a little more concise, feel free to use the extend() function!

for i in range (0,freq):
list_letters.append(letter)

#return list_letters

hand = []
b = len(list_letters)
while len(hand)<10:
index = random.randint(0,b-1)

Choose a reason for hiding this comment

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

Just a small thing, but you're using b as an upper bound here. Since you know you'll be using it as the upper bound in the randint function, it's just ever so slightly better to include the -1 to the calculation on 75 rather than within the randint parameters!

new_letter = list_letters[index]
if hand.count(new_letter) < LETTER_POOL[new_letter]:

Choose a reason for hiding this comment

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

Great use of the count function here!

hand.append(new_letter)
return hand

# Make all letter in word Capital

Choose a reason for hiding this comment

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

If you're going to include comments to detail what your function is going to do, it is preferred to include it as a docstring within the function itself as opposed to traditional comments outside. You can denote a docstring by using three ` marks:

 ```
 Make all letter in word Capital
 function returns True or False
 ```

# function returns True or False

def uses_available_letters(word, letter_bank):
pass

def score_word(word):
pass
#letter_bank = draw_letters()

Choose a reason for hiding this comment

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

Don't forget to get rid of unused comments!

copy_letter_bank = letter_bank[:]

Choose a reason for hiding this comment

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

Great way of copying the letter_bank!

word_up = word.upper()

for letter in word_up:
if letter not in copy_letter_bank or word_up.count(letter) > copy_letter_bank.count(letter):

Choose a reason for hiding this comment

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

Another great use of the count method. This particular compound conditional could get a little hard to read, so it could be separated into two different if statements if you wanted. You could also create a boolean to hold the result of word_up.count(letter) > copy_letter_bank.count(letter), but only if you wanted to increase readability!

return False
return True


# create a dictionary with points for each letter

Choose a reason for hiding this comment

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

Same as before, either add these comments as a docstring within the function or feel free to remove them entirely!

# create new variable total_score, assign 0 to it
# for each letter add corresponding points from score dictionary
# if len(word) >= 7 add 8 points
# function returns score

def score_word(word):
total_score = 0
word_up = word.upper()
for letter in word_up:
total_score += score_dict[letter]
if len(word_up) >= 7 and len(word_up)<= 10:

Choose a reason for hiding this comment

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

Not an issue, but is there a way to convert this compound conditional into one continuous conditional?

total_score += 8
return total_score

# create a dictionary with key as a word and value as a score
# iterate through the dictionary using items() method to get max_score
def get_highest_word_score(word_list):
pass
words_score_dict = {}
for word in word_list:
words_score_dict[word] = score_word(word)

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 from a previous wave!


# Look for max score, create variable max_score

Choose a reason for hiding this comment

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

Typically if we have in line comments, we'll want to keep the indentation of the comments the same as the code below them!

# create a list of words with max score in case there are several words with same score
max_score = 0
max_score_words = []
for word, w_score in words_score_dict.items():
if w_score > max_score:
max_score = w_score
max_score_words = [word]
elif w_score == max_score:
max_score_words.append(word)
# print(max_score_words)
# if there's one word with max score

if len(max_score_words) == 1:
return (max_score_words[0], max_score)

# the case when there are several words with the same score
elif len(max_score_words) > 1:

best_score_word = max_score_words[0]

for word in max_score_words:
if len(word) == 10:
best_score_word = word
return (best_score_word, max_score)

elif len(word)<len(best_score_word):
best_score_word = word
return (best_score_word, max_score)
return (best_score_word, max_score)

Choose a reason for hiding this comment

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

Overall, this code works fine, and separating everything out into individual dictionaries and lists can really help when making a first pass. You get to see each step and break out the logic quite a bit! When it comes to refactoring, however, it is possible to decrease the space complexity while maintaining similar time complexity.

With an algorithm like this, we don't need to store every score and word, just the highest. If you would like to refactor, see if you can use the same logic without holding on to every piece of data. Figuring out what we DON'T need can be just as important as what we do! Specifically, we don't need to hold onto all the words with the max score. We can make the comparisons as we score each word in the list!