Phoenix - Dana Delgado#43
Conversation
apradoada
left a comment
apradoada
left a comment
There was a problem hiding this comment.
GREEN!
Well done, Dana! Your code passes all the tests and it looks great! You should be super proud of completing your first Ada project! None of the suggestions I left are required, but you are welcome to implement them if you'd like! If you have any questions, don't hesitate to reach out!
| letter_list = [] | ||
|
|
||
| # Go through each letter and its quantities in the letter pool | ||
| for letter, quantity in LETTER_POOL.items(): |
There was a problem hiding this comment.
Great job with taking the distribution of letters into account! Great use of the extend method! This is short, sweet and concise and works really well!
| letter_list.pop(index) | ||
|
|
||
| # Return the 10 drawn letters | ||
| return hand_of_ten_letters |
There was a problem hiding this comment.
Great job on this function! Everything looks great, and it works really well! The one thing to note is that you include a comment before pretty much every line of code. While this is great for understanding your code, it can start to busy things up. I would recommend either placing the comments in a docstring at the beginning of the function as a quick write up of how the function/ algorithm works or as comments to kick off chunks of code rather than before each line!
|
|
||
| # Covert all letters to uppercase | ||
| word = word.upper() | ||
| letter_bank = [letter.upper() for letter in letter_bank] |
|
|
||
| # Create an empty dictionary to count the letters in the letter bank | ||
| available_letters = {} | ||
| for letter in letter_bank: |
There was a problem hiding this comment.
One small thing to think about is that you are creating a dictionary holding all the available letters along with their count. You are creating that dictionary from the letter bank list that you so beautifully created using a list comprehension. My thought here though is that you are not destroying the list you've created in any way, just using the values. Therefore, the list comprehension on line 60 becomes redundant and adds a for loop that doesn't need to exist. We could simply run the letter.upper() on each letter in this for loop and add it to the dictionary rather than include a whole new list comprehension! If we were instead using that comprehension to copy over the letter_bank and then destroying it later on, I could see a use, but for now it just adds an extra for loop!
| if letter in available_letters and available_letters[letter] > 0: | ||
| available_letters[letter] -= 1 | ||
| else: | ||
| return False |
There was a problem hiding this comment.
Given what you've got, this for loop looks great!
| pass | ||
|
|
||
| # Create a dictionary based on the score chart | ||
| letter_values = { |
There was a problem hiding this comment.
When we've got a hard coded dictionary like this, its usually better served as a global variable! Especially if that dictionary was needed for other functions in an expanded version of this project!
| total_score += letter_values[letter] | ||
|
|
||
| # Add bonus pointsif the world lenght is equal or greater to 7 | ||
| if len(word) >=7: |
There was a problem hiding this comment.
Small thing, but we have one other constraint on the length of the word! It can't go over 10 letters! If you want to, see if you can add that constraint to this condition!
| elif len(word) < len(highest_word) and len(highest_word) <10: | ||
| highest_word = word | ||
| # If both are the same lenght, choose the one that shows first in the list | ||
| elif len(word) == len(highest_word): |
There was a problem hiding this comment.
The only thing this condition does is continue on to the next iteration of the for loop. Since this is the last condition that would be checked before the loop moves to its next iteration, it actually isn't needed! It should work the same way with or without it!
| # Do nothing and keep the current highest word | ||
| continue | ||
|
|
||
| return (highest_word, highest_score) No newline at end of file |
There was a problem hiding this comment.
Small not, but when we're returning a tuple like this, we actually don't need the parentheses! You are more than welcome to put them in if you'd like but the code will work the same way even if they aren't present!
| # Get the score of the current word | ||
| score = score_word(word) | ||
|
|
||
| # If this score is higher than what we've got |
There was a problem hiding this comment.
This function and all the logic that goes along with it looks great! I love the way you only focused on words that were of equal or greater value and anything else was ignored! As mentioned before, the one change I would make is to watch your comments! While helpful in the early stages of programming, comments before every line aren't the norm in most work places, so feel free to get in the habit of trying to limit your comments to code blocks or a docstring!
2 participants
No description provided.