I know this code is not very 'ruby'

ruby-talk
1

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

best,

tim

···

--
Posted via http://www.ruby-forum.com/.

2

Compressed version :smiley:

discount_out = [0,40,100,160][discount_category].to_i

···

On Tue, May 6, 2008 at 9:55 AM, Tim Booher <ads@theboohers.org> 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

best,

tim
--
Posted via http://www.ruby-forum.com/\.

--
Go outside! The graphics are amazing!

3

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
  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.

Ellie

Eleanor McHugh
Games With Brains
http://slides.games-with-brains.net

···

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

----
raise ArgumentError unless @reality.responds_to? :reason

4

You already have a hash structure...

def discount category
  #I'll set up the hash for demonstration
  #You would probably use class instance
  #variables instead, or a const for your
  #id to category mapping outside of the def
  (h = Hash[1, 40, 2, 100, 3, 160]).default = 0

  #grab the category
  h[category]
end

...in which case you could do away with the def altogether depending
on what you're trying to do.

Todd

···

On Tue, May 6, 2008 at 4:55 AM, Tim Booher <ads@theboohers.org> 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

best,

tim

5

> 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

···

2008/5/6 Eleanor McHugh <eleanor@games-with-brains.com>:

On 6 May 2008, at 10:55, Tim Booher wrote:

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

6

Robert Klemme wrote:

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]

I don't like this.

instead of

discount :foo

But I like that.

So, let's do a Classic version (pardon the pun):

class Discounts

~ attr_reader :friend, :vet, :super_vet

~ def initialize
~ @friend, @vet, @super_vet = 40, 100, 160
~ end

~ def method_missing
~ 0 # The default 'discount'
~ end
end

discount_for = Discounts.new

discount_for vet
=> 100

discount_for somebody_else
=> 0

Beware of bugs; I've only proved the code correct, not tested it.

- --
Phillip Gawlowski
Twitter: twitter.com/cynicalryan
Blog: http://justarubyist.blogspot.com

~ - You know you've been hacking too long when...
...you discover that you're balancing your checkbook in octal.

7

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

That's what happens when I write code before my second cup of tea of the morning :wink:

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

I'd probably use a Hash as well for a trivial mapping like this, but I thought it was more instructive to just abolish the unnecessary variable seeing as Tim clearly didn't realise that Ruby uses a case expression rather than a case statement. And of course if the discount were to rely on several parameters then the case expression form might well be much easier to maintain.

Ellie

Eleanor McHugh
Games With Brains
http://slides.games-with-brains.net

···

On 6 May 2008, at 15:48, Robert Klemme wrote:

2008/5/6 Eleanor McHugh <eleanor@games-with-brains.com>:

----
raise ArgumentError unless @reality.responds_to? :reason

8

Phillip Gawlowski wrote:

So, let's do a Classic version (pardon the pun):

class Discounts

~ attr_reader :friend, :vet, :super_vet

~ def initialize
~ @friend, @vet, @super_vet = 40, 100, 160
~ end

~ def method_missing
~ 0 # The default 'discount'
~ end
end

discount_for = Discounts.new

discount_for vet
=> 100

discount_for somebody_else
=> 0

Beware of bugs; I've only proved the code correct, not tested it.

Well, not even that bit of due diligence. *sighs*

Obviously, that doesn't work.

discount_for.vet # that does work with the above code.

However, defining a singleton method would provide what I like to see.

def discounts_for buyer

~ case buyer
~ when friend : 40
~ .
~ else 0
~ end
end

Or define it as a class method:

class Discounts
~ # Handling of variables left out as exercise for the reader
~ def self.for
~ case[...]
~ end
end

That way you could call, for example:

Discounts.for vet

- --
Phillip Gawlowski
Twitter: twitter.com/cynicalryan
Blog: http://justarubyist.blogspot.com

~ - You know you've been hacking too long when...
...you send E-mail and end each line with \n.