RubyKoans: Greed dice scoring

Question

The Greed game, from Ruby Koans, where you roll up to 5 dice:

A set of three ones is 1000 points. A set of three numbers (other than ones) is worth 100 times the number.

A one (that is not part of a set of three) is worth 100 points. A five (that is not part of a set of three) is worth 50 points. Everything else is worth 0 points.

I broke my solution into four methods.

def score(dice)
 (1..6).reduce(0) { |sum, n| sum + points(dice, n) }
end
def points(dice, num)
 ((dice.count(num) / 3) * triple(num)) + ((dice.count(num) % 3) * bonus(num))
end
def triple(num)
 num == 1 ? 1000 : (num * 100)
end
def bonus(num)
 [1, 5].include?(num) ? 50 + (50 * (num % 5)) : 0
end

I suspect there's a way to avoid having to pass the dice array into points(), but I can't figure it out. Maybe using a yield somewhere?

Answer 1

I think your code is very nice, succinct, and ruby-like.

Two minor issues -

Your bonus code is a bit over-sophisticated, which makes it not very readable, and quite brittle. Although it won't fit in one line - a case solution will be more suitable here:

def bonus(num)
 case num
 when 1 then 100;
 when 5 then 50;
 else 0;
 end
end

Also, in score I personally think that separating the score calculation from its aggregation is more easy on the eyes:

(1..6).map { |n| points(dice, n) }.inject(:+)

although, I admit, it is more a matter of taste than anything else...

---

As for your suspicion about not having to pass dice to points, I don't see a straight-forward way of doing that, or a proper motivation for doing it.

Perhaps you want to be able to simplify your block into a straight forward method call (using a syntax like .map(&some_method)) - for that you can use proc's curry method, but it actually results in a very awkward code, which does not seem to be worth it:

def score(dice)
 (1..6).map(&lambda(&method(:points)).curry[dice]).inject(:+)
end

Perhaps you want to decouple the point calculation methods from the dice array altogether. This can be done easily enough by passing dice.count(n) instead of dice, as that is all the points actually cares about, although this solution will break as soon as point calculation will need more information...

def score(dice)
 (1..6).reduce(0) { |sum, n| sum + points(n, dice.count(n)) }
end
def points(num, count)
 ((count / 3) * triple(num)) + ((count % 3) * bonus(num))
end

• ivan: I agree on all counts! When I figured out a way to implement bonus() with modulus in a one-liner, I couldn't help myself. The case statement definitely makes more sense though. Thanks for the review.