From: Robert Klemme <shortcutter@...>
Date: 2008-05-06T23:48:09+09:00
Subject: Re: I know this code is not very 'ruby'

2008/5/6 Eleanor McHugh <eleanor@games-with-brains.com>:
>
> On 6 May 2008, at 10:55, Tim Booher wrote:
>
> > This function should work, but when I wrote it, I had the feeling that
> > there would be a much better way to code this function. In particular, I
> > don't let creating the discount_out variable. anyone want to give me a
> > nudge in the right direction?
> >
> > def discount(discount_category)
> >   #    0 => no discount
> >   #    1 => friend
> >   #    2 => vet
> >   #    3 => super-vet
> >   case discount_category
> >   when 1
> >     discount_out = 40
> >   when 2
> >     discount_out = 100
> >   when 3
> >     discount_out = 160
> >   else
> >     discount_out = 0
> >   end
> >   discount_out
> >  end
> >
>
>
>  Remember that case statements are expressions, hence:
>
>  def discount(discount_category)
>         case discount_category
>         when 1 :        40
>         when 2 :        100
>         when 3 :        160
>         else            discount_out

I guess this should read

else 0

>         end
>  end
>
>  will result in the chosen value be returned by the function.
>
>  For clarity you also might like to pretty this up further with some
> symbols:
>
>  def discount discount_category = :no_discount
>         case discount_category
>         when :friend :          40
>         when :vet :             100
>         when :super_vet :       160
>         else                            discount_out
>         end
>  end
>
>  then you don't need the comments to document your code.

I was going to suggest something similar, i.e. use more verbose
category "names".  If categories are read from somewhere they could be
mapped on input.

But, as Todd also mentions, this cries for a Hash.  With a Hash method
#discount is basically superfluous.

DISCOUNT = Hash.new(0).update(
  :friend => 40,
  :vet => 100,
  :super_vet => 160
).freeze

Then a method call essentially becomes

DISCOUNT[:foo]

instead of

discount :foo

Kind regards

robert


-- 
use.inject do |as, often| as.you_can - without end