I know this code is not very 'ruby'

Ruby
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

2

Compressed version :smiley:

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

3

On 6 May 2008, at 10:55, Tim B. wrote:

3 => super-vet

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

raise ArgumentError unless @reality.responds_to? :reason

4

2008/5/6 Eleanor McHugh [email protected]:

1 => friend

discount_out = 0

    when 1 :        40
    when 2 :        100
    when 3 :        160
    else            discount_out

I guess this should read

else 0

    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

5

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert K. 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 G.
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.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkggeIYACgkQbtAgaoJTgL9FBwCfbEkHYMGYwGyyjig79uEtpJEJ
pt8AniofvYQwftnufso/za+DfgfTPNLY
=+K4V
-----END PGP SIGNATURE-----

6

On 6 May 2008, at 15:48, Robert K. wrote:

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:

: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

raise ArgumentError unless @reality.responds_to? :reason

7

On Tue, May 6, 2008 at 4:55 AM, Tim B. [email protected] wrote:

case discount_category

end

best,

tim

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

8

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Phillip G. 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 G.
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.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEUEARECAAYFAkgggrwACgkQbtAgaoJTgL+aQgCWMqrQV0LqpvMEdFCb1yCluEKg
bACgnmMR8XvQzfoYalnQiH9ra3teVL4=
=hs10
-----END PGP SIGNATURE-----