Skip to content

Tigers- Selam Chaka #60

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

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

Tigers- Selam Chaka #60

wants to merge 12 commits into from

Conversation

s-chaka
Copy link

@s-chaka s-chaka commented Sep 29, 2022

Completed waves.

Copy link

@apradoada apradoada left a comment

Choose a reason for hiding this comment

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

Overall this is a well done project! My biggest suggestion would be to make sure you aren't using unnecessary lists or dictionaries when the calculations can be done in place! Also, don't be afraid to use deepcopy!

if letter_freq[letter] <= LETTER_POOL[letter]:
selected_letters.append(letter)
return (selected_letters[0:10])

Choose a reason for hiding this comment

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

This looks good, overall but remember that it is not accounting for the different probabilities of each letter! We could take that into account by creating a collection of some sort that holds every possible value (12 E's, 9 A's, etc) and then making our selections from that collection and removing them as they are picked!

elif temp_word != word:
return False
elif temp_word == word:
return True

Choose a reason for hiding this comment

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

I see exactly what y'all were doing here. The problem is that you are making a variety of extra dictionaries and strings that just aren't necessary. In cases like this, it would be much better to make a deep copy of the letter_bank. From there, you can simply manipulate your copy.

The process would be as follows:

  1. Make a deep copy of the letter bank.
  2. Go through each letter in the word and see if it is in the letter bank copy.
    a) If it is, remove it from the letter bank copy.
    b) If it isn't return False
  3. If you make it through the word without returning False, return True.

This will take into account multiples of the same letter!

elif temp_word != word:
return False
elif temp_word == word:
return True

Choose a reason for hiding this comment

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

I see exactly what y'all were doing here. The problem is that you are making a variety of extra dictionaries and strings that just aren't necessary. In cases like this, it would be much better to make a deep copy of the letter_bank. From there, you can simply manipulate your copy.

The process would be as follows:

  1. Make a deep copy of the letter bank.
  2. Go through each letter in the word and see if it is in the letter bank copy.
    a) If it is, remove it from the letter bank copy.
    b) If it isn't return False
  3. If you make it through the word without returning False, return True.

This will take into account multiples of the same letter!

score_word += 8

return score_word

Choose a reason for hiding this comment

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

This looks great! It's a simple and effective implementation of the score method!


return (highest_word, highest_value)

Choose a reason for hiding this comment

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

Once again, you are creating several extra lists that we don't necessarily need. This works and is a great first pass, but we could definitely pare it down a bit! One implementation would have been to have a tuple that just holds the highest scoring word and the highest score. From there, we can do the following:

  1. Make a tuple to hold the first word and its score.
  2. For each word in the list, score it and compare it to the score in your highest word tuple.
  3. If the score is higher, replace the tuple with the new word.
  4. If it is a tie, run the if/else statements you have there toward the bottom of the function!

This really just avoids having to write a tie_word list. We can kind of figure out what the highest word in place.

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