Maggie Tice- Panthers #71
Conversation
… function passing most tests and wave four function not passing any tests.
There was a problem hiding this comment.
Nice work on your first pair project, Maggie and Billy 🎉 All your tests are passing and your code is laid pretty well. I have made comments on how to refactor these approaches so they are simpler.
If you have any questions about the feedback, please reach out to me!
| quantity_data = {1: ['Z', 'X', 'K', 'Q', 'J'], | ||
| 2: ['B','C','F','H','M', 'P','V','W','Y'], | ||
| 3: ['G'], | ||
| 4: ['D', 'L', 'S', 'U'], | ||
| 6: ['N', 'R', 'T'], | ||
| 8: ['O'], | ||
| 9: ['A', 'I'], | ||
| 12: ['E']} | ||
|
|
||
| value_data = {1: ['A', 'E', 'I', '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']} |
There was a problem hiding this comment.
Placed out here as a global constant, I would recommend using all caps naming: QUANTITY_DATA and VALUE_DATA.
adagrams/game.py
Outdated
| def draw_letters(): | ||
| pass | ||
| # letter_soup = [] | ||
|
|
||
| # for data in alphabet_data_list: | ||
| # runner = 0 | ||
| # while runner < data['quantity']: | ||
| # letter_soup.append(data['letter']) | ||
| # runner += 1 | ||
|
|
||
| user_letter_pool = [] | ||
|
|
||
| while len(user_letter_pool) < 10: | ||
| draw = random.randint(0, (len(todays_soup)-1)) | ||
| drawn_letter = todays_soup[draw] | ||
|
|
||
| if user_letter_pool.count(drawn_letter) < todays_soup.count(drawn_letter): | ||
| user_letter_pool.append(drawn_letter) | ||
|
|
||
| return user_letter_pool |
There was a problem hiding this comment.
Rather than using a list of the letters in the hand, could we build a helper data structure (like a dictionary) that could let us look up if a letter was part of a user's hand?
If the helper dictionary had keys that were the letters and the values were the number of letters in the hand, then you could leverage the fact that key look up in dictionaries is a constant O(1) operation.
You can check if key (the letter in the hand) in helper_dictionary, which has constant look up time. Something like this:
def draw_letters():
letter_pool_copy = copy.deepcopy(LETTER_POOL)
letters = []
while len(letters) < 10:
letter = random.choice(list(letter_pool_copy.keys()))
if letter_pool_copy[letter] > 0:
letters.append(letter)
letter_pool_copy[letter] -= 1
return letters
|
|
||
| alphabet_data_list = generate_alphabet_data_list(quantity_data, value_data) | ||
| #makes a list that generates the numbers of letters so its proportionate to the probability of getting a certain letter | ||
| def letter_soup(letter_data): |
There was a problem hiding this comment.
You could simplify this program if you added a second argument to random.choices to select a letter that is weighted by the quantity amount.
| 10: ['Q', 'Z']} | ||
|
|
||
| #Takes the two inputs and stores them in one dictionary | ||
| def generate_alphabet_data_list(quantity_data, value_data): |
There was a problem hiding this comment.
Nice work using a custom helper function. I see that you are creating a new dictionary for the data above but why not create the data structure from scratch? Also why are we combining the data sets? We would want to handle the data separately.
adagrams/game.py
Outdated
| def uses_available_letters(word, letter_bank): | ||
| pass | ||
|
|
||
| letter_count = {} | ||
|
|
||
| # loop through word, convert all letters to uppercase, add count to letter_count | ||
| for l in word: | ||
| letter = l.upper() | ||
| if letter not in letter_count: | ||
| letter_count[letter] = 0 | ||
|
|
||
| letter_count[letter] += 1 | ||
| l_count = letter_count[letter] | ||
|
|
||
| if l_count > letter_bank.count(letter): | ||
| return False | ||
|
|
||
| return True | ||
|
|
There was a problem hiding this comment.
You could simplify this function by making a copy of the. letter bank and iterating over the words. Once we are iterating over the words we can add a conditional to find out if the letter in the words is in the letter bank copy. Something like this:
for letter in word.upper():
if letter in letter_bank_copy:
letter_bank_copy.remove(letter)
else:
return False
| def score_word(word): | ||
| pass | ||
| if type(word) is not str: | ||
| return 0 | ||
|
|
||
| score = 0 | ||
| if len(word) > 6: | ||
| score += 8 | ||
|
|
||
| word_breakdown = [] | ||
|
|
||
| for l in range(len(word)): | ||
| word_breakdown.append(word[l].upper()) | ||
|
|
||
| for i in range(len(word_breakdown)): | ||
| for data in alphabet_data_list: | ||
| if data['letter'] == word_breakdown[i]: | ||
| score += data['value'] | ||
|
|
||
| return score |
There was a problem hiding this comment.
I suggest we create a score variable. You could simplify this by creating a sum variable that starts as an integer of 0 and then add the score based on the length of the word. We can iterate over the word and add to the score based on the value of the letter in the word. Let's rethink this solution. We can also combine conditionals in one by checking if the length of the word is greater than 6 and less than 11.
|
|
||
| return score | ||
|
|
||
| # Finds highest score in list of words | ||
| def get_highest_word_score(word_list): |
There was a problem hiding this comment.
I suggest this approach: We could create the tuple we have to return at the beginning. You can grab the first word in the word_list and set that as the first value in the tuple. The second value in the tuple would be 0 as a started score.
You can then reassign that tuple's value to the word with the higher score and continue that process until you capture the highest score. You would finally return that variable 👍🏾
No description provided.