Skip to content

Sphinx - Christelle Nkera#27

Open
scnkera wants to merge 12 commits into
Ada-C22:mainfrom
scnkera:main
Open

Sphinx - Christelle Nkera#27
scnkera wants to merge 12 commits into
Ada-C22:mainfrom
scnkera:main

Conversation

@scnkera

@scnkera scnkera commented Sep 19, 2024

Copy link
Copy Markdown

No description provided.

@scnkera scnkera changed the title Sphinx - Christelle N. Sphinx - Christelle Nkera Sep 19, 2024
Comment thread adagrams/game.py

def draw_letters():
pass
LETTER_POOL = {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice job on using the constant variable naming convention for Python here!

Comment thread adagrams/game.py
Comment on lines +15 to +17
for letter, original_letter_count in LETTER_POOL.items():
for letter_index in range(original_letter_count):
all_available_let.append(letter)

@mikellewade mikellewade Sep 20, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yes, this makes sure that the letters have their weighted distribution and ensures letters like Z and E don't have the same chances of being chosen! ⭐️

Comment thread adagrams/game.py
current_hand.append(all_available_let[available_let_list_index])
all_available_let.pop(available_let_list_index)

return current_hand

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice work on this function!

Comment thread adagrams/game.py

def uses_available_letters(word, letter_bank):
pass
word = word.upper() # capitalizes input word

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice work on ensuring that all the letters where of the same casing. You could also use casefold, which is used for caseless matching. You can find more information on the method here.

Also, be mindful of re-assigning the parameter of a function. That could cause issues for the caller of the function if the change is not expected. Usually we want to avoid this unless specifically told to.

Comment thread adagrams/game.py
Comment on lines +31 to +33
for input_letter in word:
letter_freq_in_letter_bank = letter_bank.count(input_letter)
letter_freq_in_word = word.count(input_letter)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Under the hood, .count() is essentially looping over the iterable that provided it and counts the instances of its provided elements (input_letter). Essentially you are looping over word twice but this same functionality could be provided without looping over word with count. How do you think this could be done?

Comment thread adagrams/game.py
Comment on lines +43 to +48
points_dict = {
'A': 1, 'B': 3, 'C': 3, 'D': 2, 'E': 1, 'F': 4, 'G': 2, 'H': 4,
'I': 1, 'J': 8, 'K': 5, 'L': 1, 'M': 3, 'N': 1, 'O': 1, 'P': 3,
'Q': 10, 'R': 1, 'S': 1, 'T': 1, 'U': 1, 'V': 4, 'W': 4, 'X': 8,
'Y': 4, 'Z': 10
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Love this data structure, this allows us to avoid making a nested loop later on in the function.

Comment thread adagrams/game.py
if len(word) >= 7:
total_points += BONUS_POINTS

return total_points

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

very concise function!

Comment thread adagrams/game.py
Comment on lines +68 to +93
for word in word_list:
word_score = score_word(word)
word_length = len(word)

# selects the highest scoring word in the word_list
if word_score > best_score:
best_word = word
best_score = word_score
best_word_length = word_length

# selects best word according to the adagrams tie breaking rules
elif word_score == best_score:

# chooses the 10 letter word as the best word
if word_length == 10 and best_word_length != 10:
best_word = word
best_word_length = word_length

# chooses the first 10 letter word
elif word_length == 10 and best_word_length == 10:
continue

# selects the shortest word if not word length is fewer than 10 letters
elif word_length < best_word_length and best_word_length != 10:
best_word = word
best_word_length = word_length

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great job on this loop and breaking down the tie-breaking logic. There is a way that you re-arrange your if/else statements to where they can be un-nested and be more concise. Two of them will only have pass and break in them as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants