Conversation
audreyandoy
left a comment
audreyandoy
left a comment
There was a problem hiding this comment.
Great work Alf and Ivette!
Your submission covers all the learning goals and passes all the tests! Overall, the code was very clean and easy to read. This project is definitely worthy of a green grade 🟢 🌲✨
I added comments and compliments primarily on neat solutions and ways to refactor. My main feedback is to be cautious of creating separate iterations through the same collection. This creates more steps to execute, resulting in a slower-performing program. Conditionals are our best friends for checking data values, don't be afraid to combine them. More friends the better in this case 😄
Keep up the great work 🌲 ✨
| 'Y': 2, | ||
| 'Z': 1 | ||
| } | ||
|
|
There was a problem hiding this comment.
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).
| SCORE_CHART = { | ||
| 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.
Love how y'all used the score as the key in this dictionary, very clever!
| letter_hand = [] | ||
| letter_pool_list = [] | ||
| for letter, number in LETTER_POOL.items(): | ||
| letter_pool_list.extend([letter] * number) |
There was a problem hiding this comment.
Great use of extend to create a list of letters according to their letter count in the LETTER_POOL. You can omit the brackets around letter as well and it will create that same single-dimension list. Was the intention to create a nested list?
| letter_pool_list.extend([letter] * number) | |
| letter_pool_list.extend(letter * number) |
|
|
||
| def uses_available_letters(word, letter_bank): | ||
| pass | ||
| word_checker = letter_bank.copy() |
There was a problem hiding this comment.
Good job making a copy to avoid the side-effects of altering the letter bank!
| num_points += score | ||
| if len(word) >= 7: | ||
| num_points += 8 | ||
| return num_points |
There was a problem hiding this comment.
This function is so clean due to how the points and letters were stored in SCORE_CHART. Great work!!
| for word in word_list: | ||
| score = score_word(word) | ||
| words_and_scores.append((word, score)) |
There was a problem hiding this comment.
Great use of the score_word function and storing tuples in a list!
| highest_score = 0 | ||
| for pair in words_and_scores: | ||
| if pair[1] > highest_score: | ||
| highest_score = pair[1] | ||
|
|
||
| high_score_words = [] | ||
| for pair in words_and_scores: | ||
| if pair[1] == highest_score: | ||
| high_score_words.append(pair) |
There was a problem hiding this comment.
Instead of making two separate loops to iterate through the same collection, we can combine the logic into one! Doing so will reduce the number of total iterations the algorithm would execute, resulting in a (slightly) faster algorithm.
Generally, if we have to iterate through the same collection multiple times, we might as well try to find a way to combine the logic (in this case conditionals) into a single loop. Kind of like grabbing all the groceries from your car at once rather than going back and doing multiple trips 🤣 , it saves you time!
This will require one additional line of reassigning high_score_words into creating a list with a new pair containing the highest score.
| highest_score = 0 | |
| for pair in words_and_scores: | |
| if pair[1] > highest_score: | |
| highest_score = pair[1] | |
| high_score_words = [] | |
| for pair in words_and_scores: | |
| if pair[1] == highest_score: | |
| high_score_words.append(pair) | |
| highest_score = 0 | |
| high_score_words = [] | |
| for pair in words_and_scores: | |
| if pair[1] > highest_score: | |
| highest_score = pair[1] | |
| high_score_words = [pair] | |
| elif pair[1] == highest_score: | |
| high_score_words.append(pair) |
| for pair in high_score_words: | ||
| if len(pair[0]) == 10: | ||
| return pair | ||
|
|
||
| shortest_length = 10 | ||
| for pair in high_score_words: | ||
| if len(pair[0]) < shortest_length: | ||
| shortest_length = len(pair[0]) | ||
|
|
||
| for pair in high_score_words: | ||
| if len(pair[0]) == shortest_length: | ||
| return pair |
There was a problem hiding this comment.
We can reduce this section into one loop as well. However, this would require the pair with the shortest word length to be stored and later reassigned if the next iteration contained a shorter word.
| for pair in high_score_words: | |
| if len(pair[0]) == 10: | |
| return pair | |
| shortest_length = 10 | |
| for pair in high_score_words: | |
| if len(pair[0]) < shortest_length: | |
| shortest_length = len(pair[0]) | |
| for pair in high_score_words: | |
| if len(pair[0]) == shortest_length: | |
| return pair | |
| shortest_length = 10 | |
| shortest_pair = () | |
| for pair in high_score_words: | |
| if len(pair[0]) == 10: | |
| return pair | |
| elif len(pair[0]) < shortest_length: | |
| shortest_length = len(pair[0]) | |
| shortest_pair = pair | |
| elif len(pair[0]) == shortest_length: | |
| return pair | |
| return shortest_pair |
|
|
||
| def score_word(word): | ||
| pass | ||
| word = word.lower() |
There was a problem hiding this comment.
Nitpick/Opinion/🧅: If you can keep letter cases and data types consistent throughout a program then absolutely try to! Storing capital letters in SCORE_CHART would have eliminated the need for this line.
However, this is good forecasting for working with APIs/external sources of data. Oftentimes, the data we retrieve from other sources are not in the data structure we expect resulting in transforming the data into the correct letter case or data type for our program (like y'all did here).
| for i in range(10): | ||
| letter = random.choice(letter_pool_list) | ||
| letter_hand.append(letter) | ||
| letter_pool_list.remove(letter) |
There was a problem hiding this comment.
Good work in adding the remove method to ensure that the letters being picked are valid!
3 participants
No description provided.