[lnkForumImage]
TotalShareware - Download Free Software
Confronta i prezzi di migliaia di prodotti.
Asp Forum
 Home | Login | Register | Search 


 

Forums >

comp.lang.ruby

I know this code is not very 'ruby'

Tim Booher

5/6/2008 9:56:00 AM

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

Sandro Paganotti

5/6/2008 10:20:00 AM

0

Compressed version :D

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



--
Go outside! The graphics are amazing!

Eleanor McHugh

5/6/2008 10:21:00 AM

0

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
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-...
----
raise ArgumentError unless @reality.responds_to? :reason

Todd Benson

5/6/2008 2:27:00 PM

0

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

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

Robert Klemme

5/6/2008 2:48:00 PM

0

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

Phillip Gawlowski

5/6/2008 3:26:00 PM

0

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

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

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

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

Eleanor McHugh

5/6/2008 3:58:00 PM

0

On 6 May 2008, at 15:48, Robert Klemme wrote:
> 2008/5/6 Eleanor McHugh <eleanor@games-with-brains.com>:
>> 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 ;)

> 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-...
----
raise ArgumentError unless @reality.responds_to? :reason

Phillip Gawlowski

5/6/2008 4:10:00 PM

0

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

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

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

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