Ruby Koans 182. Refactor help

Can you help me refactor the solution I came up with for Ruby Koans #182? This is the koan in which you write a score method to calculate points for the Greed game. Following code works and all tests pass.

However, it feels long and un-ruby like. How can I make it better?

def score(dice)
rollGreedRoll = Hash.new
rollRollCount = Hash.new
(1..6).each do |roll|
    rollGreedRoll[roll] = roll == 1 ? GreedRoll.new(1000, 100) :
            GreedRoll.new(  100 * roll, roll == 5 ? 50 : 0)
    rollRollCount[roll] = dice.count { |a| a == roll }
en开发者_高级运维d


  score =0 


  rollRollCount.each_pair do |roll, rollCount|
    gr = rollGreedRoll[roll]  
    if rollCount < 3
        score += rollCount * gr.individualPoints
    else
        score += gr.triplePoints + ((rollCount - 3) * gr.individualPoints)

    end 
  end

  return score
end

class GreedRoll
    attr_accessor :triplePoints
    attr_accessor :individualPoints

    def initialize(triplePoints, individualPoints)
        @triplePoints = triplePoints
        @individualPoints = individualPoints
    end
end

I've put up a walkthrough of refactorings at https://gist.github.com/1091265. Final solution looks like:

def score(dice)
  (1..6).collect do |roll|
    roll_count = dice.count(roll)
    case roll
      when 1 : 1000 * (roll_count / 3) + 100 * (roll_count % 3)
      when 5 : 500 * (roll_count / 3) + 50 * (roll_count % 3)
      else 100 * roll * (roll_count / 3)
    end
  end.reduce(0) {|sum, n| sum + n}
end

note: .reduce is a synonym for .inject

You can put the rollRollCount inside the first "each", can't you? Then you don't have to iterate over the (1..6) twice.

Here's another take on it, extracting the method into its own class. A little long winded, but easy to read and understand:

def score(dice)
  GreedScore.new(dice).calculate
end

And the implementation:

class GreedScore
  def initialize(dice)
    @values = dice.sort
  end

  def calculate
    @score = 0
    score_triples
    score_singles
    @score
  end

  private

  def score_triples
    (1..6).each do |match|
      if @values.count(match) >= 3
        @score += match * (match == 1 ? 1000 : 100)
        @values = @values.drop(3)
      end
    end
  end

  def score_singles
    @values.each do |value|
      @score += 100 if value == 1
      @score += 50 if value == 5
    end
  end
end

Here was my approach. I used a hash for performance, assuming the number of dice can be large. Plus I love using inject wherever possible.

def score(dice)
  tally = 0

  return tally if dice.length == 0

  hash = dice.inject(Hash.new(0)) { |h,v| h[v] += 1; h }

  (1..6).collect do |roll|
    case roll
    when 5
      tally += (hash[roll] / 3) * 500 + (hash[roll] % 3) * 50
    when 1
      tally += (hash[roll] / 3) * 1000 + (hash[roll] % 3) * 100
    else
      tally += (hash[roll] / 3) * roll * 100
    end
  end

  ap "dice = #{dice}, " + "hash = #{hash}, " + "tally = #{tally}"

  tally
end