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

Maple- Shay & Sabrina #35

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

Conversation

SabrinaLauredan
Copy link

No description provided.

Copy link
Collaborator

@spitsfire spitsfire left a comment

Choose a reason for hiding this comment

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

Great job, Sabrina and Shay! Your code was easy to read! I made a few suggestions to dry up your code, but all in all this was nicely done!!

Keep it up!

Comment on lines +11 to +23
LETTERS_POOL = []
for letter in LETTERS_DICT:
letters_string = letter * LETTERS_DICT[letter]
for letter in letters_string:
LETTERS_POOL.append(letter)
'''
LETTERS_POOL = ['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']
Copy link
Collaborator

Choose a reason for hiding this comment

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

this works! Though I think since you put in the effort of coding the algorithm out, you should show it off! Maybe we can do something like this:

Suggested change
LETTERS_POOL = []
for letter in LETTERS_DICT:
letters_string = letter * LETTERS_DICT[letter]
for letter in letters_string:
LETTERS_POOL.append(letter)
'''
LETTERS_POOL = ['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']
def get_letters_pool:
for letter in LETTERS_DICT:
letters_pool = []
letters_string = letter * LETTERS_DICT[letter]
for letter in letters_string:
letters_pool.append(letter)
return letters_pool
LETTERS_POOL = get_letters_pool()

Copy link
Collaborator

Choose a reason for hiding this comment

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

this way, if the letter scores or availability change, then you only need to change the original dictionary!

"L": 1, "O": 1, "N": 1, "Q": 10, "P": 3, "S": 1,
"R": 1, "U": 1, "T": 1, "W": 4, "V": 4, "Y": 4,
"X": 8, "Z": 10}

def draw_letters():
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 short and sweet! love it!

a letter more times than it appears in the letter_bank
Returns True or False
'''
temp_letter_bank = 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.

👍 good job reducing side effects!

Comment on lines +57 to +61
total = 0
if len(word) >= 7:
total += 8
total += sum(SCORES[letter] for letter in word.upper())
return total
Copy link
Collaborator

Choose a reason for hiding this comment

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

ohh interesting doing the +8 points first, then using sum() for the for loop!

temp_letter_bank.remove(letter)
else:
return False
return True

def score_word(word):
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Comment on lines +75 to +77
winning_word = word
highest_score = score_word(word)
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe we can dry this up a bit, so we only use score_word once!

Suggested change
if score_word(word) > highest_score:
winning_word = word
highest_score = score_word(word)
current_score = score_word(word)
if current_score > highest_score:
winning_word = word
highest_score = current_score

Comment on lines +78 to +81
if len(word) == 10 and len(word) != len(winning_word):
winning_word = word
highest_score = score_word(word)
Copy link
Collaborator

Choose a reason for hiding this comment

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

now that we have the current_score variable, we can do the same to this conditional statement as above!

elif len(word) < len(winning_word) and len(winning_word) != 10:
winning_word = word
highest_score = score_word(word)
Copy link
Collaborator

Choose a reason for hiding this comment

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

here too!

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.

4 participants