# I know this code is not very 'ruby'

**URL:** <https://www.ruby-forum.com/t/i-know-this-code-is-not-very-ruby/136781>\
**Category:** Ruby\
**Created:** [May 6, 2008, 11:55am UTC](https://www.ruby-forum.com/t/i-know-this-code-is-not-very-ruby/136781 "2008-05-06T11:55:41Z")\
**Posts on this page:** 8\
**Page:** 1

<div class="post-metadata">

**Author:** ![tbobker](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/t/73ab20/32.png) [@tbobker](https://www.ruby-forum.com/u/tbobker)\
**Post date:** [May 6, 2008, 11:55am UTC](https://www.ruby-forum.com/t/i-know-this-code-is-not-very-ruby/136781/1 "2008-05-06T11:55:41Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![tbobker](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/t/73ab20/32.png) [@tbobker](https://www.ruby-forum.com/u/tbobker)\
**Post date:** [May 6, 2008, 12:21pm UTC](https://www.ruby-forum.com/t/i-know-this-code-is-not-very-ruby/136781/2 "2008-05-06T12:21:42Z")

</div>

Compressed version 😃

discount\_out = [0,40,100,160][discount\_category].to\_i

---

<div class="post-metadata">

**Author:** ![tbobker](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/t/73ab20/32.png) [@tbobker](https://www.ruby-forum.com/u/tbobker)\
**Post date:** [May 6, 2008, 12:22pm UTC](https://www.ruby-forum.com/t/i-know-this-code-is-not-very-ruby/136781/3 "2008-05-06T12:22:54Z")

</div>

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](http://slides.games-with-brains.net)

raise ArgumentError unless @reality.responds\_to? :reason

---

<div class="post-metadata">

**Author:** ![tbobker](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/t/73ab20/32.png) [@tbobker](https://www.ruby-forum.com/u/tbobker)\
**Post date:** [May 6, 2008, 4:49pm UTC](https://www.ruby-forum.com/t/i-know-this-code-is-not-very-ruby/136781/4 "2008-05-06T16:49:11Z")

</div>

2008/5/6 Eleanor McHugh [removed\_email\_address@domain.invalid](mailto:removed_email_address@domain.invalid):

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

---

<div class="post-metadata">

**Author:** ![tbobker](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/t/73ab20/32.png) [@tbobker](https://www.ruby-forum.com/u/tbobker)\
**Post date:** [May 6, 2008, 5:27pm UTC](https://www.ruby-forum.com/t/i-know-this-code-is-not-very-ruby/136781/5 "2008-05-06T17:27:38Z")

</div>

-----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](http://twitter.com/cynicalryan)  
Blog: [http://justarubyist.blogspot.com](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](http://enigmail.mozdev.org)

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

---

<div class="post-metadata">

**Author:** ![tbobker](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/t/73ab20/32.png) [@tbobker](https://www.ruby-forum.com/u/tbobker)\
**Post date:** [May 6, 2008, 5:59pm UTC](https://www.ruby-forum.com/t/i-know-this-code-is-not-very-ruby/136781/6 "2008-05-06T17:59:25Z")

</div>

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 😉

> :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](http://slides.games-with-brains.net)

raise ArgumentError unless @reality.responds\_to? :reason

---

<div class="post-metadata">

**Author:** ![tbobker](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/t/73ab20/32.png) [@tbobker](https://www.ruby-forum.com/u/tbobker)\
**Post date:** [May 6, 2008, 4:28pm UTC](https://www.ruby-forum.com/t/i-know-this-code-is-not-very-ruby/136781/7 "2008-05-06T16:28:21Z")

</div>

On Tue, May 6, 2008 at 4:55 AM, Tim B. [removed\_email\_address@domain.invalid](mailto:removed_email_address@domain.invalid) 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

---

<div class="post-metadata">

**Author:** ![tbobker](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/t/73ab20/32.png) [@tbobker](https://www.ruby-forum.com/u/tbobker)\
**Post date:** [May 6, 2008, 6:10pm UTC](https://www.ruby-forum.com/t/i-know-this-code-is-not-very-ruby/136781/8 "2008-05-06T18:10:37Z")

</div>

-----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](http://twitter.com/cynicalryan)  
Blog: [http://justarubyist.blogspot.com](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](http://enigmail.mozdev.org)

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