Skip to content

Ada C23 Dragonfly Class - Vicky Farinango #41

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 4 commits into
base: main
Choose a base branch
from

Conversation

vfarinango
Copy link

No description provided.

…f a word can be formed using the letters in a given letter bank.
…of a given word according to the Adagrams scoring rules.
highest scoring word from a list of words and handles edge cases according to the Adagrams scoring rules.
Copy link

@mikellewade mikellewade left a comment

Choose a reason for hiding this comment

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

Nice work on Adagrams, Vicky!

Comment on lines +3 to +31
def create_letter_pool():
letter_pool = []
letter_pool.extend(["A"] * 9)
letter_pool.extend(["B"] * 2)
letter_pool.extend(["C"] * 2)
letter_pool.extend(["D"] * 4)
letter_pool.extend(["E"] * 12)
letter_pool.extend(["F"] * 2)
letter_pool.extend(["G"] * 3)
letter_pool.extend(["H"] * 2)
letter_pool.extend(["I"] * 9)
letter_pool.extend(["J"] * 1)
letter_pool.extend(["K"] * 1)
letter_pool.extend(["L"] * 4)
letter_pool.extend(["M"] * 2)
letter_pool.extend(["N"] * 6)
letter_pool.extend(["O"] * 8)
letter_pool.extend(["P"] * 2)
letter_pool.extend(["Q"] * 1)
letter_pool.extend(["R"] * 6)
letter_pool.extend(["S"] * 4)
letter_pool.extend(["T"] * 6)
letter_pool.extend(["U"] * 4)
letter_pool.extend(["V"] * 2)
letter_pool.extend(["W"] * 2)
letter_pool.extend(["X"] * 1)
letter_pool.extend(["Y"] * 2)
letter_pool.extend(["Z"] * 1)
return letter_pool

Choose a reason for hiding this comment

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

I love that you used a helper function here! Makes you code easier to debug in case of bugs, however it is not as DRY as it could be. We could make this more DRY by using a loop (recall that loops are perfect for when we want to use preform the same logic a number of times. It could look something like this:

def create_letter_pool():
  LETTERS_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
  }
  letter_list = []

  for letter, count in LETTER_POOL.items():
    letter_list.extend(list(letter) * count)
  
  return letter_list


#Randomly select 10 characters from the data structure
letter_hand = []
for i in range(10):

Choose a reason for hiding this comment

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

Since we don't use i in the looping variable, we should replace it with _. Read more here

Suggested change
for i in range(10):
for _ in range(10):


def draw_letters():
pass
#Create the data structure containing the letters and letter distribution.

Choose a reason for hiding this comment

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

Python convention is to leave a space between the # and the list character in your comment. I would also suggest that you would remove most, if not all, of these comments to make sure that your code is clean. When using comments we want to ensure that we are only leaving comments that are vital to codebase. Our goal is to write code that is readable and explicit enough to where we don't, I believe that you've achieved this goal.

Suggested change
#Create the data structure containing the letters and letter distribution.
# Create the data structure containing the letters and letter distribution.

letter_hand = []
for i in range(10):
#Generates a random integer between 0 and the last element in letter_pool list.
random_index = randint(0,len(letter_pool)-1)

Choose a reason for hiding this comment

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

Operators like < should be surrounded with white spaces. Take a look at the PEP8 style guide on white spaces. In industry, code that doesn't follow an agreed upon code styles will need to be revised to ensure the codebase is neat and consistent.

Suggested change
random_index = randint(0,len(letter_pool)-1)
random_index = randint(0,len(letter_pool) - 1)

Comment on lines +44 to +48
random_letter = letter_pool[random_index]
#Add the newly generated random letter to letter_hand list.
letter_hand.append(random_letter)
#Remove the random_letter from letter_pool
letter_pool.remove(random_letter)

Choose a reason for hiding this comment

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

We could also use .pop here to save us a line of code.

Suggested change
random_letter = letter_pool[random_index]
#Add the newly generated random letter to letter_hand list.
letter_hand.append(random_letter)
#Remove the random_letter from letter_pool
letter_pool.remove(random_letter)
random_letter = letter_pool[random_index]
letter_hand.append(letter_pool.pop(random_letter))

Comment on lines +88 to +115
letter_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
}

Choose a reason for hiding this comment

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

Since this variable isn't changing or being modified, we want to use the python convention of naming it in all uppercase. This signals to other developers that this piece of data should not be modified or re-assigned.

Suggested change
letter_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
}
LETTER_VALUES = {
"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
}

#Sum the value of element(value) of letter_values and store in word_score
word_score += letter_value
#If the length of word is 7,8,9,10 - add 8 to word_score
if len(word) == 7 or len(word) == 8 or len(word) == 9 or len(word) == 10:

Choose a reason for hiding this comment

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

We could condense this into something like

Suggested change
if len(word) == 7 or len(word) == 8 or len(word) == 9 or len(word) == 10:
if len(word) > 6:

best_score = 0
best_scoring_tuple = None

Choose a reason for hiding this comment

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

I think that it would be okay to decouple word and score here. One, it would save us the trouble of having to use tuple access syntax and, two, have best_score which holds the score of the best word. Essentially, you are storing data twice.

word_score = score_word(word)
#Add word and it's score to tuple
word_score_tuple = (word, word_score)

Choose a reason for hiding this comment

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

Like the case with best_scoring_tuple, you are storing data twice in memory (word in this case).

Comment on lines +144 to +156
#Update best_score variable to equal word_score
best_score = word_score
#Update best_scoring_tuple to that word's tuple.
best_scoring_tuple = word_score_tuple
#Check if word_score is the same as best_score
elif word_score == best_score:
best_word = best_scoring_tuple[0]
#Count the letters in word and check if they're equal to 10
if len(word) == 10 and len(best_word) != 10:
best_scoring_tuple = word_score_tuple
elif len(best_word) > len(word) and len(best_word) != 10:
best_scoring_tuple = word_score_tuple

Choose a reason for hiding this comment

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

Nice work here!

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.

2 participants