Create a gist now

Instantly share code, notes, and snippets.

Refactor ruby koan 182
# 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment