passing all four waves #110
Conversation
| 'X': 1, | ||
| 'Y': 2, | ||
| 'Z': 1 | ||
| } |
There was a problem hiding this comment.
With a variable like LETTER_POOL which is a constant, it's just slightly more pythonic to move it outside the function and make it a global constant. The only thing this would change is the need to make a copy of the dictionary inside the function.
| drawn_letters = [] | ||
| for letter in range(10): | ||
| my_current_key = random.choice(list(LETTER_POOL)) | ||
| my_current_value = LETTER_POOL.get(my_current_key) |
There was a problem hiding this comment.
We typically only use the dictionary's .get() method when we can't be sure the key is in the dictionary. On line 36, we are choosing a key that is from the dictionary so we know it's there. As a result, it would be slightly more appropriate to just use LETTER_POOL[my_current_key] here.
| if LETTER_POOL[my_current_key] == 0: | ||
| LETTER_POOL.pop(my_current_key) | ||
| return drawn_letters | ||
|
|
There was a problem hiding this comment.
Overall this function looks good, but it doesn't fully address the weighted nature of the alphabet's randomness. Your random choice picks a letter randomly but each letter has the same weight. Is there a way you could extend that dictionary into a list that holds every letter the correct number of times it appears?
| # word is a string | ||
| #letter_bank a list of ten characters | ||
|
|
||
| bank_copy = letter_bank.copy() |
There was a problem hiding this comment.
Great use of the copy function here! We definitely want to make sure our dictionary is copied over as opposed to manipulating the original dictionary! For future reference, feel free to take a look at the difference between .copy and .deepcopy. .copy will copy over references to items within the list or dictionary rather than make a copy of the lists within. This won't cause an issue for this project, but later on when working with nested data structures, you could run into problems!
|
|
||
| if not (letter.upper() in bank_copy): | ||
| return False | ||
| elif letter.upper() in bank_copy: |
There was a problem hiding this comment.
Because we really only have two options (either the letter is in bank_copy or it's not), we can just use an else statement here!
| for letter in word: | ||
|
|
||
| if not (letter.upper() in bank_copy): | ||
| return False |
| pass | ||
| score = 0 | ||
| length_of_word = len(word) | ||
| letter_value_dict = { |
There was a problem hiding this comment.
This is another dictionary we could think about moving making a global function! When we have a larger collection like this, it's possible it may be used in other functions as we expand this program.
| for alpha in word: | ||
| score += letter_value_dict[alpha.upper()] | ||
| return score | ||
|
|
There was a problem hiding this comment.
This function is short and sweet and well done!
| return (shortest_word, word_dict[shortest_word]) | ||
| else: | ||
| return (list_of_max_keys[0], word_dict[list_of_max_keys[0]]) |
There was a problem hiding this comment.
This function definitely works and it does a good job separating out each part of this particular process. Just a couple of tips. When you start getting to a lot of if statements and for loops like this, feel free to add in some whitespace between sections. That starts to separate things out a bit.
Another way to start separating it all out as well is to see if there is a way to make it all run in one loop. One loop handles all the comparisons! It's possible!
No description provided.