Karis/Melissa - Edges - Adagrams

[kimj42]

Adagrams

Congratulations! You're submitting your assignment.

Comprehension Questions

Feature	Feedback
What are the components that make up a method?	The components are the method signature which is the name of the method, parameter, block of code, and return statement. You can invoke the method using the argument that gets passed into the parameter of the method.
What are the advantages of using git when collaboratively working on one code base?	The advantage of using git when collaboratively working on one code base is that you don't have to share one machine to do the work. It also makes you add and commit to save the changes before sending it to the pair so that all your code is definitely saved.
What kind of relationship did you and your pair have with the unit tests?	It was frustrating at times when it was failing but we were able to see where the errors and the failures were. Because the methods were so specific, it was very useful to change the code to pass the test.
Does your code use any methods from the Enumerable mixin? If so, where and why was it helpful?	We used the map method to create a new array to calculate the score of each word. It is located in line 60. It helped us condense our code.
What was one method you and your pair used to debug code?	We used pry, printed out some of the steps we were doing, and used the tests/exceptions to where the code broke down.
What are two discussion points that you and your pair discussed when giving/receiving feedback from each other that you would be willing to share?	First point: when one of us got lost, we could talk through it together and that worked for both of us. We were willing to share ideas and try those out which lead to more solutions. We are both proud of each other. Second point: TDD was hard but we became more comfortable with it as we went through it and rake was really helpful figuring out what the error was.

[droberts-sea]

Adagrams

What We're Looking For

Feature	Feedback
General
Answered comprehension questions	yes
Both teammates contributed to the codebase	yes
Small commits with meaningful commit messages	yes
Code Requirements
draw_letters method
Uses appropriate data structure to store the letter distribution	yes
All tests for draw_letters pass	yes
uses_available_letters? method
All tests for uses_available_letters? pass	yes, but see inline
score_word method
Uses appropriate data structure to store the letter scores	yes
All tests for score_word pass	yes
highest_score_from method
Appropriately handles edge cases for tie-breaking logic	yes
All tests for highest_score_from pass	yes
Overall	Good job overall! This code is well-organized and easy to read, and does a good job of solving the problem at hand. There are one or two logic bugs and places where things could be cleaned up, but in general I am pretty happy with this submission. Keep up the hard work!

[droberts-sea]

This data structure makes sense for storing the pool of letters. However, laid out on one line like this it's not easy to see the distribution of letters. Something like this might be a bit easier to read:

letters = [
  "A", "A", "A", "A", "A", "A", "A", "A", "A",
  "B", "B",
  "C", "C",
  ...
]

Or, if you were feeling fancy:

letter_distribution = {
  A: 9,
  B: 2,
  C: 2,
  ...
}
letters = []
letter_distribution.each do |letter, count|
  count.times do
    letters << letter
  end
end

[droberts-sea]

There are two problems with this implementation:

1. You will never return true for a word with a repeated letter. For example,

   hand = ["E", "F", "R", "A", "E", "L", "V", "A", "D", "K"]
   uses_available_letters?("ADA", hand)
   # => false

   The reason why is that Ruby's Array#delete method will remove all matching elements from an array, not just the first match.

2. This method is destructive! Because you call delete on the input, the hand will be reduced by the letters in the input. For example:

   hand = draw_letters
   puts hand
   # => ["T", "T", "E", "F", "R", "A", "E", "L", "V", "A"]
   uses_available_letters?('tte', hand)
   puts hand
   # => ["E", "F", "R", "A", "E", "L", "V", "A"]

   This is bad because it's unexpected behavior. Nothing about "check whether this word is made up of letters in this hand" suggests that the hand will be changed in the process, but that's exactly what happens.

   The way to address this problem would be to make a copy of the hand before removing letters from it.

I will acknowledge that none of the tests checked for either of these behaviors, but that doesn't change the fact that they're bugs.

[droberts-sea]

Again, this data structure feels like a good fit here, but I think it would be more readable if you were to split it across multiple lines.

[droberts-sea]

This works! However!

On line 77, you're calling values.max inside an each loop. That means you're doing the work of finding the max value (O(n)) once for each word (n times), for a total time complexity of O(n^2). We can improve this to O(n) by moving the max outside the loop:

max_score = words_with_scores.values.max
words_with_scores.each do |word, score|
    if score == max_score
# ...

[droberts-sea]

You have included the same file twice, as adagrams.rb and lib/adagrams.rb. Probably only the lib one should be included here.