Panthers Tiffany Rogers Kallie Gannon

[Tiffany-Rogers-Dev]

No description provided.

[kelsey-steven-ada]

Congrats on completing your first pair project Tiffany & Kallie! I’ve added some suggestions & questions, let me know if there's anything I can clarify 😊

[kelsey-steven-ada]

This structure does what we want, but a reader might need to count the number of each letter in the list to know the frequencies. If we stored them as a dictionary similar to tile_points in score_word, we could see the frequencies at a glance, and could change the available number of a given letter by editing a single value.

Since tiles_org is only accessed by the draw_letters function, it would be reasonable to place this inside the function rather than as a constant. Since the structure would be created each time the function was called, we wouldn't need to make the tiles copy in draw_letters. There are tradeoffs, the structure would clutter the function some, but it keeps the data as close as possible to where it's being used, and would mean other functions couldn't access it to accidentally alter the contents.

There's a convention in many programming languages of naming constant variables with all uppercased letters. This helps readers easily identify when a variable holds a shared value that cannot or should not change. I'd recommend naming this TILES_ORG while placed here outside of a function.

[kelsey-steven-ada]

range is a solid choice here since we know how many times we need to loop. The for-loop will create a variable letter that it doesn't look like we need, so another loop option could be a while-loop that checks if hand has ten items yet:

while len(hand) < 10:

[kelsey-steven-ada]

Nice use of deepcopy to ensure we don't change the input letter_bank.

[kelsey-steven-ada]

The name word_case is a little ambiguous - without context a reader doesn't know if case means uppercase or lowercase. What's a name that could give a reader a little extra clarity about what the variable holds?

[kelsey-steven-ada]

Great to see folks practicing for/else control flows! Something to keep in mind with for/else is that the else block will run as long as we do not encounter a break statement in the loop. Here, we have no break that will exit the loop early, so the else will always run if the loop completes without returning True.

In this implementation, we could remove the else and outdent line 30 and our execution should stay the same - we'll only return False if we left the loop without returning True on line 28.

    for char in word_case:
        if char in new_letter_bank:
            correct_letters.append(char)
            new_letter_bank.remove(char)

        if len(word_case) == len(correct_letters):
            return True 

    return False

[kelsey-steven-ada]

This is a solid approach that uses a couple data structures like correct_letters to help track things. If we were in a system where we had limited space, or processing time was crucial so we wanted to exit the loop as early as possible if an invalid character is found, then we could flip the order of our return statements. We could return false inside the loop as soon as we find any invalid character and return True at the end if no issues were found. A possible implementation could look like:

new_letter_bank = copy.deepcopy(letter_bank)

for char in word.upper():
    if char in new_letter_bank:
        # The character is valid, remove it from `new_letter_bank` so it can't be used twice
        new_letter_bank.remove(char)        
    else:
        # We found an invalid character, exit immediately
        return False


# If we exited the for loop without returning `False`, then all the
# characters in `word` must be valid and we can return `True`
return True

[kelsey-steven-ada]

This is a matter of personal preference and what's most readable for you, but since we only need upper_word in the for-loop, we could call upper() on word in the for-loop definition and remove the upper_word variable:

word_points = 0
for letter in word.upper():
    if letter in tile_points:
        word_points += tile_points[letter]

if len(word) >= 7:
    word_points += 8

[kelsey-steven-ada]

Solid approach! If we were in a system where it was important to reduce how many times we loop through the input, we could keep a variable that holds the current highest scored word we've found and tiebreak as we go:

def get_highest_word_score(word_list):
    highest = (word_list[0],0)

    for word in word_list:
        score = score_word(word)

        # If `word`'s score is greater than what's held by `highest`, updated `highest`
        if score > highest[1]:
            highest = (word, score)

        # If we have a tie, follow tie-breaking guidelines
        elif score == highest[1]:
            # The scores are tied and we've already found a 10 letter word
            # No need to update `highest`
            if len(highest[0]) == 10:
                continue # move to next iteration of the loop immediately

            # The scores are tied, `word` is 10 letters, and `highest` holds a string with less than 10 letters
            # Update highest word
            elif len(word) == 10 and len(highest[0]) != 10:
                highest = (word, score)

            # The scores are tied, neither string is 10 letters and `word` is shorter than the string held by `highest`
            # Update highest word
            elif len(word) < len(highest[0]):
                highest = (word, score)

    return highest

Another approach could be to make a single pass over the input word_list to create a list of all the words with tied max scores, then loop over the new list and apply tie-breaking rules.

[kelsey-steven-ada]

I would strongly consider adding in short comments that explain what each of the loops in this function are doing. This would help readers understand what actions the code is taking without needing to take as much time closely reading the code.