Phoenix class - Aksana#34
Conversation
apradoada
left a comment
apradoada
left a comment
There was a problem hiding this comment.
Great Job, Aksana! This project is Green!
I just left a few things to think about, but you don't have to refactor anything unless you would like to! If you have any questions about any of my feedback, don't hesitate to reach out!
| 'Z': 1 | ||
| } | ||
|
|
||
| score_dict = { |
There was a problem hiding this comment.
Just a small thing, but remember that the naming convention for a global variable would be all caps! (SCORE_DICT = )
| # create a new list from 10 random letters | ||
| list_letters = [] | ||
|
|
||
| for letter, freq in LETTER_POOL.items(): |
There was a problem hiding this comment.
Great job creating a list to account for the different letter frequencies! In the future, if you want to make something like this a little more concise, feel free to use the extend() function!
| hand = [] | ||
| b = len(list_letters) | ||
| while len(hand)<10: | ||
| index = random.randint(0,b-1) |
There was a problem hiding this comment.
Just a small thing, but you're using b as an upper bound here. Since you know you'll be using it as the upper bound in the randint function, it's just ever so slightly better to include the -1 to the calculation on 75 rather than within the randint parameters!
| while len(hand)<10: | ||
| index = random.randint(0,b-1) | ||
| new_letter = list_letters[index] | ||
| if hand.count(new_letter) < LETTER_POOL[new_letter]: |
There was a problem hiding this comment.
Great use of the count function here!
| hand.append(new_letter) | ||
| return hand | ||
|
|
||
| # Make all letter in word Capital |
There was a problem hiding this comment.
If you're going to include comments to detail what your function is going to do, it is preferred to include it as a docstring within the function itself as opposed to traditional comments outside. You can denote a docstring by using three ` marks:
```
Make all letter in word Capital
function returns True or False
```
| word_up = word.upper() | ||
|
|
||
| for letter in word_up: | ||
| if letter not in copy_letter_bank or word_up.count(letter) > copy_letter_bank.count(letter): |
There was a problem hiding this comment.
Another great use of the count method. This particular compound conditional could get a little hard to read, so it could be separated into two different if statements if you wanted. You could also create a boolean to hold the result of word_up.count(letter) > copy_letter_bank.count(letter), but only if you wanted to increase readability!
| word_up = word.upper() | ||
| for letter in word_up: | ||
| total_score += score_dict[letter] | ||
| if len(word_up) >= 7 and len(word_up)<= 10: |
There was a problem hiding this comment.
Not an issue, but is there a way to convert this compound conditional into one continuous conditional?
| pass No newline at end of file | ||
| words_score_dict = {} | ||
| for word in word_list: | ||
| words_score_dict[word] = score_word(word) |
There was a problem hiding this comment.
Great use of the score_word() function from a previous wave!
| for word in word_list: | ||
| words_score_dict[word] = score_word(word) | ||
|
|
||
| # Look for max score, create variable max_score |
There was a problem hiding this comment.
Typically if we have in line comments, we'll want to keep the indentation of the comments the same as the code below them!
| elif len(word)<len(best_score_word): | ||
| best_score_word = word | ||
| return (best_score_word, max_score) | ||
| return (best_score_word, max_score) |
There was a problem hiding this comment.
Overall, this code works fine, and separating everything out into individual dictionaries and lists can really help when making a first pass. You get to see each step and break out the logic quite a bit! When it comes to refactoring, however, it is possible to decrease the space complexity while maintaining similar time complexity.
With an algorithm like this, we don't need to store every score and word, just the highest. If you would like to refactor, see if you can use the same logic without holding on to every piece of data. Figuring out what we DON'T need can be just as important as what we do! Specifically, we don't need to hold onto all the words with the max score. We can make the comparisons as we score each word in the list!
2 participants
Adagrams project submission, wrote 4 functions.