-
Notifications
You must be signed in to change notification settings - Fork 33
Leaves - Natalie, Nicky, Samantha #22
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
base: master
Are you sure you want to change the base?
Conversation
AdagramsWhat We're Looking For
|
require "pry" | ||
|
||
def draw_letters | ||
alphabet_soup = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider making this hash a global variable using all caps that is defined outside the method.
|
||
def uses_available_letters?(input, letters_in_hand) | ||
dup_array = letters_in_hand.dup | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you are returning the value returned from a block of code, explicitly assign that value to a variable, and then return that variable.
input.upcase.split(//).all? do |letter|
included = dup_array.include?(letter)
dup_array.delete(letter)
end
return included
score = 0 | ||
|
||
word_array = word.upcase.split(//) | ||
word_array.each do |letter| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this case
statement works, it means that the information about which letter has which score is locked into this piece of code, and can't easily be used elsewhere. For example, if you wanted to display the value of each letter in a hand, you would need to repeat this work.
An alternative approach would be to store the letter scores in a hash, something like this:
LETTER_SCORES = {
"A" => 1
"B" => 3,
"C" => 3,
"D" => 2,
# ...
}
Then to get the score for a letter, you can say LETTER_SCORES[letter]
.
winners = new_hash.select { |word, score| score == max_score } | ||
winners_array = winners.keys | ||
|
||
winners_array.each do |word| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic here is a bit complicated and you still have not passed that one test:
test_0006_in case of tied score, prefers most the word with 10 letters regardless of order FAIL (0.00s)
Expected: "AAAAAAAAAA"
Actual: "BBBBBB"
It seems that if instead of nesting if/else control structures a chained conditional would simplify your logic. Here is such a chained conditional for a tiebreaker between 2 words.
def tiebreaker(word1, word2)
if word1.length == 10
return word1
elsif word2.length == 10
return word2
elsif word1.length < word2.length
return word1
elsif word1.length > word2.length
return word2
else
word1
end
end
winners = new_hash.select { |word, score| score == max_score } | ||
winners_array = winners.keys | ||
|
||
winners_array.each do |word| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using comments to outline the logic of the tiebreakers rules
Adagrams
Congratulations! You're submitting your assignment.
Comprehension Questions
Enumerable
mixin? If so, where and why was it helpful?