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

Lion Calss-Kimia Hasani-C18 #69

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

Conversation

kimiahasani
Copy link

hi Auberon, I'm Kimia, As you know, Carina was my partner , She said, her feeling really unwell , and need to stay home We I work on my project by my self
wave 4 was difficult for me.

you are the best teacher ever
I learn a lot form you
Thank you so much

Copy link

@alope107 alope107 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! You've got some awesome logic here. In the future, consider having your commit messages describe what changes you made instead of just what waves passed, and consider having more frequent commits. Nicely done, this project is green!

Comment on lines 32 to +45
def draw_letters():
pass
#empty list to hold keys
all_letters =[]
#empty list to hole random letter
letter_bank =[]

for key, value in LETTER_POOL.items():
for i in range(value):
all_letters.append(key)
while len(letter_bank) < 10:
letter = random.choice(all_letters)
letter_bank.append(letter)
all_letters.remove(letter)
return letter_bank

Choose a reason for hiding this comment

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

Awesome logic here! In the future, try to use more descriptive variable names. Instead of key and value, you could say something like letter and repetitions.

def draw_letters():
pass
#empty list to hold keys
all_letters =[]

Choose a reason for hiding this comment

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

Small style nit: add space after =

Comment on lines 48 to +68
def uses_available_letters(word, letter_bank):
pass
#word is string describes some input word
#letter_bank is list of drawn leters in hand(10 string)
#if every letter in input is avilable in letter_bank return Tru
#if not letter in input in letter bank or has to much compare to letter bank return False
upper_letter = word.upper()
dict = {}
for letter in letter_bank:
# print(letter)
if letter in dict:
dict[letter]+=1
dict[letter]=1

for letter in upper_letter:
if letter in dict and dict[letter] > 0:
dict[letter] -=1
continue
else:
return False

return True

Choose a reason for hiding this comment

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

I really like the idea here! However, there's a bug for words with repeated letters. The loop from lines 55-59 will always set each letter in the word to have a value of 1, even if it shows up multiple times. Line 59 needs to be enclosed in an else to avoid overwriting the updated value.

Smaller stylistic notes: line 64 is unneeded and try to use more descriptive variable names than dict.

Comment on lines +99 to +104
score = 0
for letter in word.upper():
score += SCORE_CHART[letter]
if len(word) > 6:
score += 8
return score

Choose a reason for hiding this comment

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

Great logic!

Comment on lines +112 to +114
score = score_word(word_list[word])
word_and_score[word_list[word]] = score

Choose a reason for hiding this comment

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

Consider not using a range here. We could just do for word in word_list: and then we wouldn't need to index into the list inside the loop.

Comment on lines +118 to +133
if not best_word_and_score:
best_word_and_score.append(word)
best_word_and_score.append(score)
elif score > best_word_and_score[1]:
best_word_and_score[1] = score
best_word_and_score[0] = word

elif score == best_word_and_score[1] and len(word)< len(best_word_and_score[0]) and len(best_word_and_score[0]) != 10:
best_word_and_score[1] = score
best_word_and_score[0] = word

elif score == best_word_and_score[1] and len(word) == 10 and len(word) != len(best_word_and_score[0]):
best_word_and_score[1] = score
best_word_and_score[0] = word
return tuple(best_word_and_score)

Choose a reason for hiding this comment

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

Great logic! Instead of having best_word_and_score be a list of length 2, consider just having it be two different variables. Then, combine the two variables into a tuple at the end. That way we don't have to remember which is in index 0 and which is in index 1 in the list as we're performing out logic.

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