Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cedar-Jacy and Bailey <3 #29

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

beaniebaye
Copy link

No description provided.

Copy link
Collaborator

@beccaelenzil beccaelenzil left a comment

Choose a reason for hiding this comment

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

Nice work! You've worked through some tricky logic in clever ways and clearly met the learning goals. I've left a few minor comments on ways you might consider refactoring, but overall great work!

}

# creates list of letter distribution in adagrams game, returns list
def build_letter_pool():
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice use of a helper function.

def draw_letters():
pass
#build a list of all the letters
Copy link
Collaborator

Choose a reason for hiding this comment

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

Clear logic and good use of comments to clarify even further!

Comment on lines +71 to +72
for char in word:
if char.upper() in letter_bank_copy:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider refactoring to reassign char = char.upper() to make it even more clear you're making this function case insensitive.

###########################################################

#make constant_dict for letter_score
LETTER_SCORE = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider moving this data structure to the top so that all of your constants are in one place.

for char in word:
total_score += LETTER_SCORE[char.upper()]

# add 8 to total score if letter len is >= 7
Copy link
Collaborator

Choose a reason for hiding this comment

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

The comments on line 121 and 125 are almost identical to your code. While guiding comments can enhance the readability of your code, these types of comments are ok to leave out (or good to remove if they were there as pseudo-code)

# Wave 4 #
###########################################################

# returns highest_scoring_word and max_score
def get_highest_word_score(word_list):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function is clear and logical. You might consider breaking out the tie breaking logic into a helper function to further enhance readability.

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.

3 participants