seeking advice on Ruby code style

Discussion in 'Ruby' started by Alexandru E. Ungur, Jan 11, 2007.

  1. Hi all,

    I recently recently released my first vim script, nothing big, it
    just toggles words between true/false, on/off, etc. You can find
    more here http://vim.sourceforge.net/scripts/script.php?script_id=1748
    if you want.

    The thing is, I'm not sure if I chose the most elegant/beautiful/**
    way to solve the problem, basically I ended up deciding between
    two ways of handling the problem:

    class String
    @@pairs = [ %w[on off], %w[yes no], %w[true false] ]

    def toggle_word1
    pair = @@pairs.select{|p| p.include?(downcase)}.flatten
    return nil if pair.empty?
    antiword = pair[pair.index(downcase) ^ 1]
    case self
    when upcase then return antiword.upcase
    when downcase then return antiword.downcase
    when capitalize then return antiword.capitalize
    else return antiword
    end
    end

    def toggle_word2
    pair = @@pairs.select{|p| p.include?(downcase)}.flatten
    wordcase = %w[upcase downcase capitalize].detect {|c| send(c) == self} || "downcase"
    pair.empty? ? nil : pair[pair.index(downcase) ^ 1].send(wordcase)
    end
    end

    Both toggle_word1 and toggle_word2 are doing the exact same thing, that is:
    "Yes".toggle_word1 # => "No"
    "No".toggle_word2 # => "Yes"
    and so on.

    My question is, which is more elegant, more... rubyful ?
    I like that toggle_word1 is almost like plain english, especially
    that "case" construct, but I like in toggle_word2 that the same "case"
    construct was replaced with a single line of code, well with two
    actually. However perhaps toggle_word2 is a little too much
    code golfing...?


    Thank you in advance for your comments,
    Have a nice day everyone,
    Alex
     
    Alexandru E. Ungur, Jan 11, 2007
    #1
    1. Advertising

  2. On Thu, Jan 11, 2007 at 10:49:29PM +0900, Alexandru E. Ungur wrote:
    > I recently recently released my first vim script, nothing big, it
    > just toggles words between true/false, on/off, etc. You can find
    > more here http://vim.sourceforge.net/scripts/script.php?script_id=1748
    > if you want.

    [...]

    Here's a cleaner implementation (assuming I understand it correctly):

    class String
    PAIRS = Hash[*%w(on off yes no true false)]
    PAIRS.to_a.each { |v,k|
    v.freeze
    k.freeze
    PAIRS[k] = v
    }
    PAIRS.freeze

    def toggle_word
    return self unless antiword = PAIRS[downcase]
    case self
    when upcase
    antiword.upcase
    when downcase
    antiword.downcase
    when capitalize
    antiword.capitalize
    else
    antiword
    end
    end

    end

    Note that I am avoiding class variables (which are almost always the wrong
    choice, see
    http://www.oreillynet.com/ruby/blog/2007/01/nubygems_dont_use_class_variab_1.html
    ) and using a constant instead. The freezing is optional, but I like to
    freeze what my constants point to. Notice that I am reversing all the
    entries in the hash so it's easy to toggle both ways. I kept the case
    statement from toggle_word1 over the golfing in toggle_word2, but I'll
    point out that Ruby only requires a return when short-circuiting.

    > Thank you in advance for your comments,
    > Have a nice day everyone,
    > Alex

    --Greg
     
    Gregory Seidman, Jan 11, 2007
    #2
    1. Advertising

  3. Alexandru E. Ungur

    Guest

    On Jan 11, 2007, at 8:49 AM, Alexandru E. Ungur wrote:
    > My question is, which is more elegant, more... rubyful ?
    > I like that toggle_word1 is almost like plain english, especially
    > that "case" construct, but I like in toggle_word2 that the same "case"
    > construct was replaced with a single line of code, well with two
    > actually. However perhaps toggle_word2 is a little too much
    > code golfing...?


    I think in both cases you are repeating too much work that could be
    done ahead of time. How about:

    class String
    base = %w[on off yes no true false]
    all = base
    all += base.map {|x| x.upcase }
    all += base.map {|x| x.capitalize }
    t = Hash[*all]
    TOGGLE = t.merge t.invert

    def toggle_word3
    TOGGLE[self] || TOGGLE[downcase]
    end
    end

    if __FILE__ == $0
    require 'test/unit'

    class My_Test < Test::Unit::TestCase
    def test_toggle_word3
    assert_equal('off', 'on'.toggle_word3)
    assert_equal('Off', 'On'.toggle_word3)
    assert_equal('YES', 'NO'.toggle_word3)
    assert_equal('yes', 'nO'.toggle_word3)
    assert_equal('false', 'true'.toggle_word3)
    assert_equal(nil, 'unknown'.toggle_word3)
    end
    end
    end


    Gary Wright
     
    , Jan 11, 2007
    #3
  4. On Thu, Jan 11, 2007 at 11:40:52PM +0900, wrote:
    [...]
    > TOGGLE = t.merge t.invert


    Yow, how did I miss Hash#invert? That's beautiful.

    --Greg
     
    Gregory Seidman, Jan 11, 2007
    #4
  5. Alexandru E. Ungur wrote:
    > Hi all,
    >
    > I recently recently released my first vim script, nothing big, it
    > just toggles words between true/false, on/off, etc. You can find
    > more here http://vim.sourceforge.net/scripts/script.php?script_id=1748
    > if you want.
    >
    > The thing is, I'm not sure if I chose the most elegant/beautiful/**
    > way to solve the problem, basically I ended up deciding between
    > two ways of handling the problem:
    >
    > class String
    > @@pairs = [ %w[on off], %w[yes no], %w[true false] ]
    >
    > def toggle_word1
    > pair = @@pairs.select{|p| p.include?(downcase)}.flatten
    > return nil if pair.empty?
    > antiword = pair[pair.index(downcase) ^ 1]
    > case self
    > when upcase then return antiword.upcase
    > when downcase then return antiword.downcase
    > when capitalize then return antiword.capitalize
    > else return antiword
    > end
    > end
    >
    > def toggle_word2
    > pair = @@pairs.select{|p| p.include?(downcase)}.flatten
    > wordcase = %w[upcase downcase capitalize].detect {|c| send(c) == self} || "downcase"
    > pair.empty? ? nil : pair[pair.index(downcase) ^ 1].send(wordcase)
    > end
    > end
    >
    > Both toggle_word1 and toggle_word2 are doing the exact same thing, that is:
    > "Yes".toggle_word1 # => "No"
    > "No".toggle_word2 # => "Yes"
    > and so on.
    >
    > My question is, which is more elegant, more... rubyful ?
    > I like that toggle_word1 is almost like plain english, especially
    > that "case" construct, but I like in toggle_word2 that the same "case"
    > construct was replaced with a single line of code, well with two
    > actually. However perhaps toggle_word2 is a little too much
    > code golfing...?
    >
    >
    > Thank you in advance for your comments,
    > Have a nice day everyone,
    > Alex


    class String
    def toggle_word3
    words = %w[on off yes no true false]
    word = words[ i = words.index(downcase) ]
    anti = words[ i + (i%2>0 ? -1 : 1 ) ]
    %w( capitalize upcase ).each{|meth|
    return anti.send(meth) if word.send(meth) == self }
    anti
    end
    end
     
    William James, Jan 11, 2007
    #5
  6. Boy, am I glad I asked for advice...
    I hesitated for a few days to ask for advice on this, since I thought:
    this is too easy, how to ask for advice on such a simple problem?, I
    mean, in how many ways you can toggle a word, right? Now I feel really
    dumb... :) as I've seen, the answer is: in many and **very**
    interesting ways.
    Simply amazing and enlightening...

    A *big* thank you to you all!
    All the best,
    Alex
     
    Alexandru E. Ungur, Jan 11, 2007
    #6
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. bill
    Replies:
    3
    Views:
    700
  2. iksrazal
    Replies:
    2
    Views:
    363
    enrique
    Apr 27, 2005
  3. Steve W. Jackson

    Seeking class hierarchy advice

    Steve W. Jackson, Apr 26, 2006, in forum: Java
    Replies:
    1
    Views:
    289
    Chris Uppal
    Apr 27, 2006
  4. Replies:
    4
    Views:
    136
    s0nspark
    May 17, 2006
  5. psema4
    Replies:
    2
    Views:
    115
    psema4
    Jul 24, 2006
Loading...

Share This Page