Help me refactor my ruby code please

Ruby
1

Right now a have a piece of code that i use in a rails app. Take a look
at it:

def calculate(days, size, terminal)
(0…days).inject {|sum, day| sum + costs(day, size, terminal)}
end

def costs(days, size, terminal)
if terminal == “PLP”
if size == 20
case days
when 0…5; 0
when 6; 75
when 7…8; 25
when 9…20; 75
else 125
end
elsif size == 40
case days
when 0…5; 0
when 6; 150
when 7…8; 50
when 9…20; 150
else 250
end
end
elsif terminal == “FCT”
if size == 20
case days
when 0…5; 0
when 6; 75
when 7…8; 25
when 9…20; 115
when 21…30; 165
when 31…60; 150
else 135
end
elsif size == 40
case days
when 0…5; 0
when 6; 150
when 7…8; 50
when 9…20; 230
when 21…30; 330
when 31…60; 300
else 270
end
end
end
end

puts calculate(14, 40, “FCT”)

The idea behind it’s quite obvious, i think. Depending on a cost per
day, container size and terminal i get the total cost.

That case statement looks ugly for me, though i can’t find a more
elegant way. And when i imagine 10 more terminals… The previous
version was like (without terminal and size):

COSTS = { 0…5 => 0, [6] => 75, [7, 8] => 25, 9…20 => 115, 21…30 =>

165, 31…60 => 250 }

COSTS.default = 0

#def calculate(days)

price = 0

(0…days).each do |day|

COSTS.each_key do |key|

price += COSTS[key] if key.include?(day)

end

end

price

#end

but here the calculate method is ugly…

Guys, do u have any idea what i could do with that? Thanks a lot =)

2

On Nov 5, 2011, at 11:30 AM, Pavel V. wrote:

 case days
   when 7..8; 50
   when 9..20; 115
   when 21..30; 330

The idea behind it’s quite obvious, i think. Depending on a cost per
#def calculate(days)

Guys, do u have any idea what i could do with that? Thanks a lot =)

Personally, I’d prefer your older version. You can put your data in a
nested hash and freeze it. Then you could use something like this
calculate:

def calculate(days, size = 40, terminal = :FCT)
(0 … days).inject do |sum, day|
sum + COSTS.deep_fetch(terminal, size, day, 0)
end
end

Although a default cost of 0 is probably not in your best interest :wink:

This uses a #deep_fetch as proposed by Avdi G. on his blog:

3

thanks a lot, Sylvester!