-
-
Save jedidja/702315 to your computer and use it in GitHub Desktop.
require File.expand_path(File.dirname(__FILE__) + '/edgecase') | |
# Greed is a dice game where you roll up to five dice to accumulate | |
# points. The following "score" function will be used calculate the | |
# score of a single roll of the dice. | |
# | |
# A greed roll is scored as follows: | |
# | |
# * A set of three ones is 1000 points | |
# | |
# * A set of three numbers (other than ones) is worth 100 times the | |
# number. (e.g. three fives is 500 points). | |
# | |
# * 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. | |
# | |
# | |
# Examples: | |
# | |
# score([1,1,1,5,1]) => 1150 points | |
# score([2,3,4,6,2]) => 0 points | |
# score([3,4,5,3,3]) => 350 points | |
# score([1,5,1,2,4]) => 250 points | |
# | |
# More scoring examples are given in the tests below: | |
# | |
# Your goal is to write the score method. | |
class RollCounter | |
def self.countNonSetRollsForDieFace(rolls, dieFace) | |
rolls.count(dieFace) % 3 | |
end | |
def self.countSetsOfThreeForDieFace(rolls, dieFace) | |
rolls.count(dieFace) / 3 | |
end | |
end | |
class GreedScoreCalculator | |
def self.calculateScoreForDieFace(dieFace, numberOfRolls) | |
if (dieFace == 1) | |
numberOfRolls * 100 | |
elsif (dieFace == 5) | |
numberOfRolls * 50 | |
end | |
end | |
def self.calculateScoreForSetOfThree(dieFace, numberOfSets) | |
if (dieFace == 1) | |
numberOfSets * 1000; | |
else | |
numberOfSets * dieFace * 100; | |
end | |
end | |
end | |
class Greed | |
def self.calculateScore(rolls) | |
score = (1..6).inject(0) do |result, dieFace| | |
result + GreedScoreCalculator.calculateScoreForSetOfThree(dieFace, | |
RollCounter.countSetsOfThreeForDieFace(rolls, dieFace)) | |
end | |
return score + | |
GreedScoreCalculator.calculateScoreForDieFace(1, RollCounter.countNonSetRollsForDieFace(rolls, 1)) + | |
GreedScoreCalculator.calculateScoreForDieFace(5, RollCounter.countNonSetRollsForDieFace(rolls, 5)) | |
end | |
end | |
def score(dice) | |
return Greed.calculateScore(dice) | |
end | |
class AboutScoringProject < EdgeCase::Koan | |
def test_score_of_an_empty_list_is_zero | |
assert_equal 0, score([]) | |
end | |
def test_score_of_a_single_roll_of_5_is_50 | |
assert_equal 50, score([5]) | |
end | |
def test_score_of_a_single_roll_of_1_is_100 | |
assert_equal 100, score([1]) | |
end | |
def test_score_of_multiple_1s_and_5s_is_the_sum_of_individual_scores | |
assert_equal 300, score([1,5,5,1]) | |
end | |
def test_score_of_single_2s_3s_4s_and_6s_are_zero | |
assert_equal 0, score([2,3,4,6]) | |
end | |
def test_score_of_a_triple_1_is_1000 | |
assert_equal 1000, score([1,1,1]) | |
end | |
def test_score_of_other_triples_is_100x | |
assert_equal 200, score([2,2,2]) | |
assert_equal 300, score([3,3,3]) | |
assert_equal 400, score([4,4,4]) | |
assert_equal 500, score([5,5,5]) | |
assert_equal 600, score([6,6,6]) | |
end | |
def test_score_of_mixed_is_sum | |
assert_equal 250, score([2,5,2,2,3]) | |
assert_equal 550, score([5,5,5,5]) | |
end | |
end |
Hmm - I wish there was a way to reply directly to a comment :)
With your inject, you are using the pattern:
inject() { |accum, item| accum.add(item); accum }
This is often a flag that there is an abstraction missing for what you are doing here. This is an incredible common pattern that often stems from primitive obsession.
Is this still an issue within the class? I suppose we could extract this part into another class that just keeps track of the DiceRolls, and have another class that deals with just scoring...
Here's a thought: do you actually need to pre-process your list of throws into the hash? Do you need to count all the faces before you start processing? What if you counted the faces as you needed them?
Regarding the abstraction, it is more the question of what this class does:
1 - counts the number of times a face appears
2 - runs the rules for scoring sets of 3 (more than one red flag here)
3 - runs the rules for scoring sets of non-3 (more than one red flag here)
I think I'm getting closer to what you're suggesting :) Also, tried to incorporate some of the "state-less" discussions from Chicago -- there doesn't seem to be any need to force the RollCounter to be initialized with the rolls when I can pass them in each time.
I did some reading on the class methods in Ruby vs. static methods in Java/C# and feel they might be appropriate here. What do you think?
My method names are starting to look like Objective-C ones though..pretty long heh.
Getting better. Still a few weird parts.
However, one immediate thing about inject:
If you don't pass a parameter, then it will use the first value in your collection as the first value. So, you can get rid of the #inject(0) and replace it with a simple #inject
If I understand your inject comment, I would write:
(0..6).inject
instead of:
(1..6).inject(0)
That feels a little odd, given that there is no "0" die face. But perhaps having a simpler statement makes up for that fact?
I think the abstractions are starting to gel a little better...
RollCounter - counts die based on face
GreedScoreCalculator - calculates score based on face
Greed - calculates score for roll
I think (heh) that at least the classes only do "one" thing each. Or at least their methods are closely aligned. And there are no methods past 3 "lines" which is an improvement from the start.
I could see the class methods being weird; simple enough to change them...what else feels wrong?
Really appreciate all the comments :)
new to ruby but though it worth firing over my version of the score method, comments welcome...
def score(dice)
# You need to write this method
return 0 if dice==[]
one = two = three = four = five = six = result = 0
dice.each do |x|
case x
when 1; one += 1
when 2; two += 1
when 3; three += 1
when 4; four += 1
when 5; five += 1
when 6; six += 1
end
end
if one > 2
result = 1000
one = one - 3
end
if two > 2
result = 100 * 2
elsif three > 2
result = 100 * 3
elsif four > 2
result = 100 * 4
elsif five > 2
result = 100 * 5
five = five - 3
elsif six > 2
result = 100 * 6
end
if one > 0
result = result + (100 * one)
end
if five > 0
result = result + (50 * five)
end
return result
end
new to ruby but though it worth firing over my version of the score method, comments welcome...
def score(dice)
# You need to write this method
return 0 if dice==[]
one = two = three = four = five = six = result = 0
dice.each do |x|
case x
when 1; one += 1
when 2; two += 1
when 3; three += 1
when 4; four += 1
when 5; five += 1
when 6; six += 1
end
end
if one > 2
result = 1000
one = one - 3
end
if two > 2
result = 100 * 2
elsif three > 2
result = 100 * 3
elsif four > 2
result = 100 * 4
elsif five > 2
result = 100 * 5
five = five - 3
elsif six > 2
result = 100 * 6
end
if one > 0
result = result + (100 * one)
end
if five > 0
result = result + (50 * five)
end
return result
end
I personnally your solution overkill, but we can argue that it depends how one apporach the excercise.
My take on this as a ruby beginner is here. Comments welcome :)
https://gist.github.com/906429
sebrech: thanks for the link :) I haven't looked at refactoring this code in ages; I do agree your solution is simpler :)
I'm guessing I still need to tackle calculateScoreForSetsOfThree though...