Refactor ruby koan 182

refactor-ruby-koan-182.rb

# http://stackoverflow.com/questions/6738715/ruby-koans-182-refactor-help

def old_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 }
  end

  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

def refactor1(dice)
  # This seems backwards but it makes things clearer later.
  # Lets temporarily seperate the rollGreedRoll
  # and rollRollCount init loops.
  rollGreedRoll = Hash.new
  (1..6).each do |roll|
      rollGreedRoll[roll] = roll == 1 ? GreedRoll.new(1000, 100) :
              GreedRoll.new(  100 * roll, roll == 5 ? 50 : 0)
  end

  rollRollCount = Hash.new
  (1..6).each do |roll|
      rollRollCount[roll] = dice.count { |a| a == roll }
  end

  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

def refactor2(dice)
  # The rollRollCount map is being used to provide (1..6)
  # and the corresponding count.  We can just compute the count
  # inside a (1..6).each so lets remove it.
  # Also introduce dice.count(obj)
  rollGreedRoll = Hash.new
  (1..6).each do |roll|
      rollGreedRoll[roll] = roll == 1 ? GreedRoll.new(1000, 100) :
              GreedRoll.new(  100 * roll, roll == 5 ? 50 : 0)
  end

  score =0

  (1..6).each do |roll|
    rollCount = dice.count(roll)
    gr = rollGreedRoll[roll]
    if rollCount < 3
        score += rollCount * gr.individualPoints
    else
        score += gr.triplePoints + ((rollCount - 3) * gr.individualPoints)

    end
  end

  return score
end

def refactor3(dice)
  # The rollGreedRoll map is only being used in one place
  # within a similiar loop. Lets remove it.
  score =0

  (1..6).each do |roll|
    rollCount = dice.count(roll)
    gr = roll == 1 ? GreedRoll.new(1000, 100) :
           GreedRoll.new(  100 * roll, roll == 5 ? 50 : 0)
    if rollCount < 3
        score += rollCount * gr.individualPoints
    else
        score += gr.triplePoints + ((rollCount - 3) * gr.individualPoints)

    end
  end

  return score
end

def refactor4(dice)
  # there are two places score gets added to
  # we can reduce that to one useing % and /
  score =0

  (1..6).each do |roll|
    rollCount = dice.count(roll)
    gr = roll == 1 ? GreedRoll.new(1000, 100) :
           GreedRoll.new(  100 * roll, roll == 5 ? 50 : 0)
    score += gr.triplePoints * (rollCount / 3) + gr.individualPoints * (rollCount % 3)
  end

  return score
end

def refactor5(dice)
  # notice a summation loop
  # we can replace that with a collect + sum/reduce
  (1..6).collect do |roll|
    rollCount = dice.count(roll)
    gr = roll == 1 ? GreedRoll.new(1000, 100) :
           GreedRoll.new(  100 * roll, roll == 5 ? 50 : 0)
    gr.triplePoints * (rollCount / 3) + gr.individualPoints * (rollCount % 3)
  end.reduce(0) {|sum, score| sum + score}
end

def refactor6(dice)
  # the GreedRoll cases seem complicated
  # so lets break that apart
  (1..6).collect do |roll|
    rollCount = dice.count(roll)
    gr = case roll
           when 1 : GreedRoll.new(1000, 100)
           when 5 : GreedRoll.new(500, 50)
           else     GreedRoll.new(100 * roll, 0)
         end
    gr.triplePoints * (rollCount / 3) + gr.individualPoints * (rollCount % 3)
  end.reduce(0) {|sum, score| sum + score}
end

def refactor7(dice)
  # the GreedRoll class is just providing two accessors
  # so let get rid of it
  (1..6).collect do |roll|
    rollCount = dice.count(roll)
    triplePoints, individualPoints = case roll
           when 1 : [1000, 100]
           when 5 : [500, 50]
           else     [100 * roll, 0]
         end
    triplePoints * (rollCount / 3) + individualPoints * (rollCount % 3)
  end.reduce(0) {|sum, score| sum + score}
end

def score(dice)
  # lets remove the triple and individual points
  # as they are only used once
  # and lets redo naming using _ vs camelCase
  (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

# At this point you could try to recollapse the scoring function
# or collapse the 5 and default cases, but I think its clearer
# what is happening this way