Feedback.md

Feedback!

Pros

• All the tests still pass
• I like your use of reduce in the chance method, good use of a proc as well.
• The determine_score_for_number method is a very nice abstraction. The method reads well and really DRYs up the codebase.

Constructive Criticism

• I see why you have a self.set_dice_array method. I like the idea of giving this method the responsbility of creating an array for us. In ruby there is a better way of taking a group of arguments and then referencing that group of arguments as an array. You can do

     is_yatzy = die.all? { |value| value == die[0] }
     determine_score(is_yatzy)
   end

and then die will be an array of your dice. Although this scenario is unlikely, doing it this way gives us the added benefit of make our code more flexible if the amount of dice used in Yahtzee game ever changed.

• I think is_yatzy? can be pulled out into a boolean method.

• This can be a longer discussion we have in person, but I would move the yatzy scoring logic into the yatzy method itself instead of having a determine_score method.

  • I think the determine_score method is a little bit misleading as a name. I would expect it to determine any score.

  • I see why you may have made this method, since you have a determine_score_for_number and you wanted to create some sort of consistent interface for the your class to interact with. If that is the case we would need to create another method called determine_score_for_chance as well. Also I would rename determine_score to determine_score_for_yatzy.

• All of your methods are public, and breaking encapsulation. Any methods that aren't being called by an outside class can be a private method.

• I may have not brought this up, but you can refactor the tests as well to make the class you are testing make more sense. For example the way of scoring .threes and #fours should be the same. So it is strange one is a instance method and the other is a class method. I would expect all of the methods in this yatzy class to be class methodssince we aren't concerned about state. We just are taking in die, and figuring out the score.

• This is nitpicky and people go either way on this but I would pull out the number 50 into a constant on the class. I would quickly research the phrase 'magic values' to understand why.