Seeking advice on Ruby code style

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

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

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

On Thu, Jan 11, 2007 at 11:40:52PM +0900, [email protected] wrote:
[…]

TOGGLE = t.merge t.invert

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

–Greg

Alexandru E. Ungur wrote:

when capitalize then return antiword.capitalize

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

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… :slight_smile: 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

This forum is not affiliated to the Ruby language, Ruby on Rails framework, nor any Ruby applications discussed here.

| Privacy Policy | Terms of Service | Remote Ruby Jobs