Snow Leopards - Elsje & Dainiz #61
Conversation
| @@ -1,5 +1,32 @@ | |||
| # AdaGrams | |||
|
|
|||
| ## Plan of Action | |||
| pass | ||
| letter_pool = [] | ||
| letter_bank = [] | ||
| for letter, count in LETTER_DISTRIBUTION.items(): |
There was a problem hiding this comment.
This works perfectly well! I have a couple of suggestions - you don't really need to turn the LETTER_DISTRIBUTION dictionary in to a list of tuples in order to access the count nor do you need to use a for-loop with an unused variable i. Here's an alternative based on your approach which doesn't convert the dictionary to a list of tuples or create unused variables:
for letter in LETTER_DISTRIBUTION:
count = LETTER_DISTRIBUTION[letter]
while count > 0:
letter_pool.append(letter)
count -= 1
| for i in range(count): | ||
| letter_pool.append(letter) | ||
| # Should we rewrite 45-47 with list comprehension? This is what we tried: | ||
| # letter_pool = [[letter] * count |
There was a problem hiding this comment.
If you want to write a list comprehension using the nested loop approach, you'd need a nested list comprehension. I speak for myself in saying I don't usually find those to be very readable.
| else: | ||
| bank_dict[item] += 1 | ||
| for letter in word.upper(): | ||
| if letter not in bank_dict.keys(): |
There was a problem hiding this comment.
Python can iterate over a dictionary and uses its keys to do so, so no need to convert the dictionary to a list of dictionary keys just for iterating over it.
| for letter in word.upper(): | ||
| if letter not in bank_dict.keys(): | ||
| return False | ||
| elif letter in bank_dict.keys(): |
There was a problem hiding this comment.
Same as above, no need to convert to a list first.
| bank_dict = {} | ||
| # We tried to implement dictionary comprehension for this, | ||
| # but are unsure if it's possible: | ||
| # alt_bank_dict = {letter, 1 for letter in letter_bank |
There was a problem hiding this comment.
It's possible! Try this:
bank_dict = {letter: letter_bank.count(letter) for letter in letter_bank}
| for tally, letters in SCORE_CHART.items(): | ||
| if character in letters: | ||
| total_score += tally | ||
| if len(word) >= 7: |
|
|
||
| def score_word(word): | ||
| pass | ||
| total_score = 0 | ||
| for character in word.upper(): |
There was a problem hiding this comment.
This does work however, if you create a data structure like:
{
'A': 1,
'B': 3,
'C': 3,
'D': 2,
...
}Then you can avoid nested iteration:
for letter in word:
score_total += LETTER_VALUES[letter]Which is much simpler and easier to read! Always consider the best data structure that allows the simplest and most straightforward access.
| word_info = [{"word": word, "score": score_word(word), "length": len(word)} |
There was a problem hiding this comment.
This function works great! One minor improvement would be to avoid looping by checking whether there's only one element in word_info. Since that list only contains more than one element if there's ties, you can use that fact to return the sole element, e.g.
if len(word_info) == 1:
return word_info[0]["word"], word_info[0]["score"]Then go on to do all the tie-break looping only if you actually have a tie.
|
Well done! Good job with adding the documentation of your pairing effort and with the general organization of your code, using lots of commits with descriptive messages, and using built-ins where it made sense. Be sure to consider your data structures using the guidelines I offered in the PR comments, because well-chosen data structures will mean simpler code when accessing them! |
We added a few comments within the code itself - let us know your thoughts!! Thanks for looking over it for us!