I know this code is not very 'ruby'

Discussion in 'Ruby' started by Tim Booher, May 6, 2008.

  1. Tim Booher

    Tim Booher Guest

    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
     
    Tim Booher, May 6, 2008
    #1
    1. Advertisements

  2. Compressed version :D

    discount_out = [0,40,100,160][discount_category].to_i
     
    Sandro Paganotti, May 6, 2008
    #2
    1. Advertisements


  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
     
    Eleanor McHugh, May 6, 2008
    #3
  4. Tim Booher

    Todd Benson Guest

    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
     
    Todd Benson, May 6, 2008
    #4
  5. I guess this should read

    else 0
    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
     
    Robert Klemme, May 6, 2008
    #5
  6. -----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.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-----
     
    Phillip Gawlowski, May 6, 2008
    #6
  7. That's what happens when I write code before my second cup of tea of
    the morning ;)
    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
     
    Eleanor McHugh, May 6, 2008
    #7
  8. -----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.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-----
     
    Phillip Gawlowski, May 6, 2008
    #8
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.