C22 Phoenix - Jenny M. #42
Conversation
There was a problem hiding this comment.
GREEN!
Great job, Jenny! Overall your code passes all the tests and it works well! I left a few comments for you, but they are simply suggestions or things to think about in the future! You are more than welcome to make any changes that you would like, but they are not required! If you do make changes, feel free to let me know and I'm more than happy to take a second look!
As usual, if you have any questions about anything I've written, don't hesitate to reach out!
| def draw_letters(): | ||
| pass | ||
|
|
||
| copy_pool = {key: value for key, value in ORIGINAL_POOL.items()} |
There was a problem hiding this comment.
This is a really great dictionary comprehension, Jenny!
| @@ -1,11 +1,123 @@ | |||
| import random | |||
|
|
|||
| ORIGINAL_POOL = { | |||
There was a problem hiding this comment.
Great creation of a global variable!
| keys = copy_pool.keys() | ||
| key = list(keys)[index] |
There was a problem hiding this comment.
Overall this works, and I get the inclination to separate out each part so you can see each step. As you get more comfortable with programming, feel free to combine these two lines into one as it is often more used in industry!
For example:
key = list(copy_pool.keys())[index]
One other thing to note is that the .keys () is being run for each iteration of the for loop, but it's being created and used the same way each time. The same thing is happening on line 39. In order to keep the computer from having to redo the creation, it might be best to create a variable to hold that list outside the loop so that it is only created once!
| if copy_pool[key] > 0: | ||
| pool.append(key) | ||
| copy_pool[key] = - 1 | ||
| return pool |
There was a problem hiding this comment.
The last consideration we have for this function is that it currently doesn't take the distribution of letters into account. Right now, every letter has an equal chance of being chosen. In reality, we have different amounts of each letter, so we would have a higher chance of picking something like an E over a Q! This wasn't tested, nor was it required, so you don't have to update your code to meet that, but if you'd like to extend your learning, that would be a great place to start!
|
|
||
| def uses_available_letters(word, letter_bank): | ||
| pass | ||
| word = word.upper() | ||
| letter_bank = [letter.upper() for letter in letter_bank] |
There was a problem hiding this comment.
Once again, a really great list comprehension! One small thing I would say is that you are copying over letter bank into a list of the same name. Just to avoid confusion, I would suggest using a name like letter_bank_copy.
| letter_bank.remove(letter) | ||
| else: | ||
| return False | ||
| return True |
There was a problem hiding this comment.
The rest of this function looks great! Well done, Jenny!
|
|
||
| def score_word(word): | ||
| pass | ||
| score_chart = [ |
There was a problem hiding this comment.
A couple of notes about this particular structure!
- This list seems like it might work better as a global variable! Especially if we were to extend this project.
- The more nested our structure becomes, the more complicated it is to parse through. I'll comment a little more below, but even though it is a little annoying to type out, having a single dictionary where each letter maps to its point value is going to be one of the best ways to approach the scores!
All that being said, I hope this helped you understand nested data structures as it can be a very useful skill!
| for letter in word.upper(): | ||
| for item in score_chart: | ||
| if letter in item["letters"]: | ||
| total += item["point"] |
There was a problem hiding this comment.
This is what I was talking about in terms of your score structure. When we factor in the fact that if letter in item["letters"]: runs using a loop, we end up with a triply nested loop, which is not ideal. If you changed up the structure to be a single dictionary, this exact same thing could be accomplished with a single loop!
Keep in mind that the strength of a dictionary is the fact that lookup is fast and easy. The more structures we nest inside the dictionary, the more we disrupt that fast and easy lookup!
| selected_word = word | ||
|
|
||
| return (selected_word, highest_score) |
There was a problem hiding this comment.
This function looks great! Great job avoiding unnecessary extra dictionaries and lists and jumping straight to the logic of the highest scores and words! One small thing is that when you return multiple values from a function, they are returned as a tuple, so we don't need to include parentheses to indicate a tuple in the return statement.
No description provided.