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

C19Ruby- Sarah Kim #19

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

C19Ruby- Sarah Kim #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d957899
added my name to the readme
suyon02kim Mar 16, 2023
6d53b37
test 1 and 2 of test_wave_01 passed
suyon02kim Mar 17, 2023
d0d1dd0
function draw_letters returns a list of random letters according to t…
suyon02kim Mar 18, 2023
de82185
refined the draw_letters function, ommitted redundant lines of code
suyon02kim Mar 18, 2023
2ffffc7
added functionality for uses_available_letters checking to see if the…
suyon02kim Mar 18, 2023
ad525c3
added all functionality to uses_available_letters
suyon02kim Mar 19, 2023
6a830b7
added functionality to letter_score where each letter will be scored …
suyon02kim Mar 21, 2023
0e87f4a
added functionality to get_highest_word_score to return the highest s…
suyon02kim Mar 22, 2023
335d4e9
refactored and added comments to the get_highest_word_score function
suyon02kim Mar 23, 2023
05e63f1
refactored and added comments to get_highest_word_score function
suyon02kim Mar 23, 2023
c971784
final version of Adagrams
suyon02kim Mar 23, 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
0 README.md → README_sarah.md
View file
Open in desktop
File renamed without changes.
119 changes: 115 additions & 4 deletions adagrams/game.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,122 @@
import random
def draw_letters():
pass
list_of_letters = []
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
suyon02kim marked this conversation as resolved.
Show resolved Hide resolved
}

while len(list_of_letters) < 10:
letter_chosen = random.choice(list(LETTER_POOL))
Copy link

Choose a reason for hiding this comment

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

This way of choosing letters randomly doesn't quite meet the requirements. Notice that your solution is just as likely to draw a Z as it is to draw an E. The requirements say, "Since there are 12 Es but only 1 Z, it should be 12 times as likely for the user to draw an E as a Z". It turns out, our tests don’t test for that. (That test is quite hard to write!) Think about how you might modify your code to fulfill this requirement.

Regarding random.choice: when bringing in functions from imported modules (that we haven’t covered in the curriculum), please consider including a comment about how you understand the function to work, and how you researched it. Now that we’ve discussed Big O, it’s also useful to think about what the time/space complexity of the function might be. Thinking about how we can implement this functionality ourselves can help us gain insight into what that might be.

Remember that not every language has the same built-in functions, so we should be able to implement this same functionality ourselves. In the projects, we don’t ask for any features that we don’t think you should be able to write yourself. For drawing a random hand, the only external function that would be “required” is a way to pick a random number (such as randint). At this stage in our coding journeys, it’s often more valuable to re-implement things by hand to practice thinking about how to break down and approach the problem than to use a single library function to solve the problem for us.

if LETTER_POOL[letter_chosen] > 0:
list_of_letters.append(letter_chosen)
LETTER_POOL[letter_chosen] -= 1

return list_of_letters

def uses_available_letters(word, letter_bank):
pass
#always make string 'word' be uppercase
word = word.upper()
#generating a dictionary to keep track of how many each letter occurs in letter bank
letter_frequency = {}
for letter in letter_bank:
if letter in letter_frequency:
letter_frequency[letter] += 1
else:
letter_frequency[letter] = 1
#compare each letter in word to the key in letter frequency dictionary
for elem in word: # loop through each letter in the word
if elem in letter_frequency.keys():
if letter_frequency[elem] > 0:
letter_frequency[elem] -= 1
else:
return False
else:
return False
Comment on lines +52 to +59
Copy link

Choose a reason for hiding this comment

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

Nice way of handling this!

[suggestion] You can make the cases slightly more concise by inverting the conditions, combining them, and early returning only in the if body. Early-returning in an if means the rest of the loop body is naturally an "else":

    for elem in word: # loop through each letter in the word 
        if elem not in letter_frequency.keys() or letter_frequency[elem] <= 0: 
            return False 
        letter_frequency[elem] -= 1

return True


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

Choose a reason for hiding this comment

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

[nit] since you don't modify letter_score, it can be extracted out of the function as a constant. You might call it LETTER_SCORE if you did that.

"A" : 1,
"E" : 1,
"I" : 1,
"O" : 1,
"U" : 1,
"L" : 1,
"N" : 1,
"R" : 1,
"S" : 1,
"T" : 1,
"D" : 2,
"G" : 2,
"B" : 3,
"C" : 3,
"M" : 3,
"P" : 3,
"F" : 4,
"H" : 4,
"V" : 4,
"W" : 4,
"Y" : 4,
"K" : 5,
"J" : 8,
"X" : 8,
"Q" : 10,
"Z" : 10
}
total_score = 0
#turn every letter in the word to upper case
Copy link

Choose a reason for hiding this comment

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

Comments are good, and sometimes you can have too much of a good thing! In most of this function, your comments are almost exactly the same as the lines of code they document. We avoid that in industry because the lines of code may change, but devs aren't always careful about changing the comments to match. It's also more for humans to read, but doesn't give them extra information.

What comments might you write to describe what this code is doing without describing how it does it (since that is what's most likely to change later)?

word = word.upper()
# loop through each letter in the string "word"
for letter in word:
# add the value of the letter to a variable called total_score
total_score += letter_score[letter]
# check if the word length is >=7 and if it is then add 8 to the total_score
if len(word) >= 7:
total_score += 8
return total_score

def get_highest_word_score(word_list):
pass
highest_scoring_word = ""
highest_score = 0
for new_word in word_list:
# find each score for word in the word_list
score_for_one_word = score_word(new_word)
#decide on tie breaker first
if score_for_one_word == highest_score:
# length of 10 letters trumps any word that is shorter
if len(new_word) == 10 and len(highest_scoring_word)!= 10:
highest_scoring_word = new_word
# having fewer letters is a tie breaker as long as the current stored word's length is not 10
elif len(highest_scoring_word) > len(new_word) and len(highest_scoring_word) != 10:
highest_scoring_word = new_word
Comment on lines +111 to +117
Copy link

Choose a reason for hiding this comment

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

[idea] If you think it's more readable (which you might not!), you can combine these cases together and rely on the order of operations rules of or and and to give you equivalent boolean logic:

        if score_for_one_word == highest_score and len(highest_scoring_word) != 10:
            if len(new_word) == 10 or len(highest_scoring_word) > len(new_word):
                highest_scoring_word = new_word

#Look for word with highest score
elif score_for_one_word > highest_score:
highest_score = score_for_one_word
highest_scoring_word = new_word
return highest_scoring_word, highest_score