Ruby Koans - Scoring Project

about_scoring_project.rb

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

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 :)