Re: Help me refactor my ruby code please

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

> Right now a have a piece of code that i use in a rails app. Take a =
look
> at it:
>=20
> def calculate(days, size, terminal)
>  (0..days).inject {|sum, day| sum + costs(day, size, terminal)}
> end
>=20
> def costs(days, size, terminal)
>  if terminal =3D=3D "PLP"
>    if size =3D=3D 20
>      case days
>        when 0..5; 0
>        when 6; 75
>        when 7..8; 25
>        when 9..20; 75
>        else 125
>      end
>    elsif size =3D=3D 40
>      case days
>        when 0..5; 0
>        when 6; 150
>        when 7..8; 50
>        when 9..20; 150
>        else 250
>      end
>    end
>  elsif terminal =3D=3D "FCT"
>    if size =3D=3D 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 =3D=3D 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
>=20
> puts calculate(14, 40, "FCT")
>=20
>=20
> The idea behind it's quite obvious, i think. Depending on a cost per
> day, container size and terminal i get the total cost.
>=20
> 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):
>=20
> # COSTS =3D { 0..5 =3D> 0, [6] =3D> 75, [7, 8] =3D> 25, 9..20 =3D> =
115, 21..30 =3D>
> 165, 31..60 =3D> 250 }
> # COSTS.default =3D 0
>=20
> #def calculate(days)
> #  price =3D 0
> #  (0..days).each do |day|
> #    COSTS.each_key do |key|
> #      price +=3D COSTS[key] if key.include?(day)
> #    end
> #  end
> #  price
> #end
>=20
> but here the calculate method is ugly...
>=20
> Guys, do u have any idea what i could do with that? Thanks a lot =3D)

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 =3D 40, terminal =3D :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 ;-)

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

http://avdi.org/devblog/2011/06/28/do-or-do-not-there-is-no-try/