Leaves - Elizabeth + Cloudy #6
Conversation
… entered word is made of letters in their hand.
…not in the drawn letters to pass appropriately, test 003 still failing, have not written method for it yet.
…ers are used that aren't in the letters_in_hand_array, tested and works.
…ie has 10 letters then it wins over the other.
AdagramsWhat We're Looking For
I'm making a few comments on this assignment about some suggestions on how to refactor. In particular, I call attention to some redundant code, suggest a way you can store letter frequency information in a data structure, and name your variables meaningfully to increase readability. Overall, you two have done a great job. Good work! |
| return letters_in_hand | ||
| end | ||
|
|
||
| ARRAY_OF_LETTERS = ["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"] |
There was a problem hiding this comment.
It's advisable to store the frequency information in a hash, and then use that hash to create an array. Also, it's best to put this global at the top of your script.
letter_counts = {
"A"=>9, "B"=>2, ...
}
letters = []
letter_counts.each do |letter, count|
count.times do
letters << letter
end
end| end | ||
| end | ||
|
|
||
| check = 0 |
There was a problem hiding this comment.
This check seems redundant. The first loop will return false if a letter in the word is not in the hand. If that return doesn't happen, you can simply return true after that loop is finished.
| "A" => 1, "B" => 3, "C" => 3, "D" => 2, "E" => 1, "F" => 4, "G" => 2, "H" => 4, "I" => 1, "J" => 8, "K" => 5, "L" => 1, "M" => 3, "N" => 1, "O" => 1, "P" => 3, "Q"=> 10, "R"=> 1, "S" => 1, "T" => 1, "U" => 1, "V" => 4, "W" => 4, "X" => 8, "Y" => 4, "Z" => 10 | ||
| } | ||
| word_array = word.upcase.chars | ||
| word_points = word_array.map do |k, v| |
There was a problem hiding this comment.
Use meaningful names for k and v (i.e. letter and letter_value).
| word_points = word_array.map do |k, v| | ||
| letter_points[k] | ||
| end | ||
| total_points = word_points.sum |
There was a problem hiding this comment.
Consider using the .sum enumerable to combine the map and sum operations your perform.
| return highest_score_hash | ||
| end | ||
|
|
||
| def tiebreaker_length (old_word, new_word) |
There was a problem hiding this comment.
Great use of a helper method. You could consider putting more of the tiebreaker functionality into this method.
Adagrams
Congratulations! You're submitting your assignment.
Comprehension Questions
Enumerablemixin? If so, where and why was it helpful?