HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

RubyKoans: Greed dice scoring

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
rubykoansscoringdicegreed

Problem

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?

Solution

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

Code Snippets

def bonus(num)
  case num
  when 1 then 100;
  when 5 then 50;
  else 0;
  end
end
(1..6).map { |n| points(dice, n) }.inject(:+)
def score(dice)
  (1..6).map(&lambda(&method(:points)).curry[dice]).inject(:+)
end
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

Context

StackExchange Code Review Q#48213, answer score: 6

Revisions (0)

No revisions yet.