Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 107 additions & 4 deletions adagrams/game.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,114 @@
import copy

import random

tiles_org = ["A", "A", "A","A", "A", "A","A", "A", "A","B", "B", "C", "C", 'D', 'D', 'D', 'D', 'E', 'E','E', 'E', 'E','E', 'E', 'E','E', 'E', 'E','E', 'F', 'F', 'G', 'G', 'G', 'H', 'H', 'I', 'I', 'I', 'I', 'I', 'I', 'I', 'I', 'I', 'J', 'K', 'L', 'L', 'L', 'L', 'M', 'M', 'N', 'N', 'N', 'N', 'N', 'N', 'O', 'O', 'O', 'O', 'O', 'O', 'O', 'O', 'P', 'P', 'Q', 'R', 'R', 'R', 'R', 'R', 'R', 'S', 'S', 'S', 'S', 'T', 'T', 'T', 'T', 'T', 'T', 'U', 'U', 'U', 'U', 'V', 'V', 'W', 'W', 'X', 'Y', 'Y', 'Z']

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.



def draw_letters():
pass
tiles = tiles_org.copy()
hand = []

for letter in range(10):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

tile = random.randint(0,len(tiles)-1)
hand.append(tiles[tile])
tiles.remove(tiles[tile])
return hand

def uses_available_letters(word, letter_bank):
pass
correct_letters = []
new_letter_bank = copy.deepcopy(letter_bank)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

word_case = word.upper()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

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
Comment on lines +19 to +28

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

else:
return False
Comment on lines +29 to +30

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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



def score_word(word):
pass
tile_points = {
'A': 1,
'E': 1,
'I': 1,
'O': 1,
'U': 1,
'L': 1,
'N': 1,
'R': 1,
'S': 1,
'T': 1,
'D': 2,
'G': 2,
'B': 3,
'C': 3,
'M': 3,
'P': 3,
'F': 4,
'H': 4,
'V': 4,
'W': 4,
'Y': 4,
'K': 5,
'J': 8,
'X': 8,
'Q': 10,
'Z': 10
}
word_points = 0
upper_word = word.upper()
for letter in upper_word:
Comment on lines +62 to +64

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if letter in tile_points:
word_points += tile_points[letter]
if len(upper_word) >= 7:
word_points += 8

return word_points

def get_highest_word_score(word_list):
pass

score_list = []

for word in word_list:
score = score_word(word)
score_list.append(score)

highest_score_index = 0
for index in range (len(word_list)):
score = score_list[index]
if score > score_list[highest_score_index]:
highest_score_index = index

for index in range (len(word_list)):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

score = score_list[index]
current_word = word_list[index]
high_score = score_list[highest_score_index]
high_score_word = word_list[highest_score_index]
if score == high_score and len(current_word) < len(high_score_word):
highest_score_index = index

for index in range (len(word_list)):
score = score_list[index]
current_word = word_list[index]
high_score = score_list[highest_score_index]
high_score_word = word_list[highest_score_index]
if score == high_score and len(current_word) == 10:
highest_score_index = index

for index in range (len(word_list)):
score = score_list[index]
current_word = word_list[index]
high_score = score_list[highest_score_index]
high_score_word = word_list[highest_score_index]
if score == high_score and len(current_word) == len(high_score_word):
highest_score_index = index
word_list[0]
break

return [word_list[highest_score_index],score_list[highest_score_index]]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.