Skip to content

Sharks, Kassidy Buslach and Juli Tera#51

Open
buslakj wants to merge 32 commits intoada-c17:masterfrom
buslakj:master
Open

Sharks, Kassidy Buslach and Juli Tera#51
buslakj wants to merge 32 commits intoada-c17:masterfrom
buslakj:master

Conversation

@buslakj
Copy link
Copy Markdown

@buslakj buslakj commented Apr 1, 2022

No description provided.

Copy link
Copy Markdown

@audreyandoy audreyandoy left a comment

Choose a reason for hiding this comment

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

Great work Kassidy and Juli!

Your submission covers all the learning goals and passes all the tests! Y'all have earned a green grade 🟢

In your PR, you'll find comments from me on ways to refactor and improve your code. Keep up the great work! 🦈✨

Comment thread adagrams/game.py
Comment on lines +4 to +17
LETTER_QUANTITY_DICT = {
'A' : 9, 'N' : 6 ,
'B' : 2, 'O' : 8,
'C' : 2, 'P' : 2,
'D' : 4, 'Q' : 1,
'E' : 12,'R' : 6,
'F' : 2, 'S' : 4,
'G' : 3, 'T' : 6,
'H' : 2, 'U' : 4,
'I' : 9, 'V' : 2,
'J' : 1, 'W' : 2,
'K' : 1, 'X' : 1,
'L' : 4, 'Y' : 2,
'M' : 2, 'Z' : 1}
Copy link
Copy Markdown

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).

Comment thread adagrams/game.py
while counter < quantity:
letter_list.append(letter)
counter += 1
letter_bank = random.sample(letter_list, 10)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good work in keeping the statistical probability in mind for building the hand of letters!

Comment thread adagrams/game.py
else:
return False

def check_letters_intersecting(word, letter_bank):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice helper function and use of sets!

Comment thread adagrams/game.py
letter_bank = random.sample(letter_list, 10)
return letter_bank

def uses_available_letters(word, letter_bank):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

Comment thread adagrams/game.py
Comment on lines +52 to +60
score_chart = {
1:["A","I","E","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"]
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We can move this data structure outside of this function as a constant to declutter this function. In addition, the majority of the data that we'll work with will often be passed in as a parameter so having this data exist outside of the function models more closely to how we'd actually build functions.

Comment thread adagrams/game.py
Comment on lines +64 to +65
if word == "":
return total_score
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just in case word is passed in as other Falsey values (0, [], False), we can make this guard clause more robust by checking for all Falsey cases rather than just an empty string.

    if not word:
        return total_score

Comment thread adagrams/game.py
Comment on lines +66 to +71
for letter in word.upper():
letter_list.append(letter)
for letter in letter_list:
for point, letters in score_chart.items():
if letter in letters:
total_score += point
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rather than creating a new list, we can directly iterate through word as strings are also iterables!

    for letter in word.upper():
        for point, letters in score_chart.items():
            if letter in letters:
                total_score += point

Comment thread adagrams/game.py
word_scores = {}
for word in word_list:
word_scores[word] = score_word(word)
highest_score_dict = {word:score for word, score in word_scores.items() if score == max(word_scores.values())}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice dictionary comprehension!!

Comment thread adagrams/game.py
Comment on lines +84 to +92
for word, score in sorted(highest_score_dict.items()):
winning_word = {}
shortest_word = min(len(x) for x in highest_score_dict.keys())
if len(word) == 10:
winning_word[word] = score
return convert_dict_to_tuple(winning_word)
elif len(word) == shortest_word:
winning_word[word] = score
return convert_dict_to_tuple(winning_word)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In this case, we only need to find the shortest word in the dictionary once rather than with every iteration so we can move this to be above the loop.

In addition, the default iterator for dictionaries in python3 is its keys. We can omit keys() and experience the same behavior.

        shortest_word = min(len(x) for x in highest_score_dict.keys())
        for word, score in sorted(highest_score_dict.items()):
            winning_word = {}
            if len(word) == 10:
                winning_word[word] = score
                return convert_dict_to_tuple(winning_word)
            elif len(word) == shortest_word:
                winning_word[word] = score
                return convert_dict_to_tuple(winning_word)

Copy link
Copy Markdown

@audreyandoy audreyandoy Apr 16, 2022

Choose a reason for hiding this comment

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

While I love helper functions, we actually don't need top make one to return a tuple. python3 automatically returns a tuple when multiple values separated by a comma are in a return statement (return word, score):

  for word, score in sorted(highest_score_dict.items()):
            winning_word = {}
            if len(word) == 10:
                winning_word[word] = score
                return word, score 
            elif len(word) == shortest_word:
                winning_word[word] = score
                return word, score

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants