def my_method
@foo == :bar ? :foo_matches : nil
end
with
def my_method
:foo_matches if @foo == :bar
end
doesn't look like an improvement to me. The first style is direct and readable, and lowers the barrier of entry to understand it. The second one requires to be familiar with the subtleties of Ruby, which adds difficulty when you're working with multiple languages daily or getting started with Ruby. I'm not saying you should avoid pushing Ruby to its best capabilities so that it's readable by any programmer, but in this case, winning 4 characters doesn't seem worth the compromise.
If you're coming from another programming language, sure, you know what ternary operators are already and can reason out what the function does without necessarily knowing Ruby that well, but read the logic out loud:
Ternary: "If instance variable foo equals symbol bar, then return symbol foo_matches, else return nil"
Ruby Idiom: "Return symbol foo_matches if instance variable foo equals symbol bar"
To me, I don't see it as saving characters as much as the ternary version feeling less idiomatic. In the ternary version you've got to convert 5 sigils into meaning in your head to reason out that method, plus the ? and : sigils as ternary operators are less frequently used in Ruby anyways, so you may have to pause a bit longer on them since the ? and colon are frequently used for query-style methods and symbols respectively. That adds context as a requirement to interpreting them in your head. The idiomatic version is functionally equivalent, but reads a little better converted to English. You've only got to convert 4 programming language sigils into meaning and there's no ambiguity of the colon sigil and the ? goes away entirely.
I hear you, and I realize Rubyists tend to avoid ternary operators, but I think this case is one where idiomatic Ruby makes unnecessary compromises against readability. Your arguments still don't convince me the latter is more readable.
Ruby Idiom: "Return symbol foo_matches if instance variable foo equals symbol bar"
You forgot the "else, it doesn't get called and the method returns nil by default" part. Although it's implicit, it doesn't mean you don't have to reason about it. On top of that, being implicit raises the barrier of understanding.
you may have to pause a bit longer on them since the ? and colon are frequently used for query-style methods and symbols respectively
I copy pasted from the article but I agree that readability could be improved with parentheses:
(@foo == :bar) ? :foo_matches : nil
I don't think the colon is a problem for readability either, being surrounded by spaces. I'll give you that a programmer coming from another language may make the mistake to think the space is optional.
The ternary is generally a problem for readability. Ruby's particular style (like being able to return a value from an if-block) generally make the cases where they're useful obsolete.
In this case, if we cared about readability, we wouldn't use a ternary at all:
if @foo == :bar
:foo_matches
else
nil
end
Which simplifies down to the exact refactoring offered.
In Kent Beck's Smalltalk Patterns book he suggest that although every method returns "self" if you actually intend for that value to be used you should return it explicitly. I believe it's called the "Interesting Return Value Pattern' and I think the same applies here, the nil should be explicit.
Agreed, this is an example where returning true or false would be more useful. I presumed the point was assuming a situation where a method returning either a value or nil would be desirable.
I am just not a fan of that pattern of testing the return value of an assignment statement:
if (( info = details[:info] ))
...do something
instead of
info == details[:info]
if info
... do something
It always looks like somebody typed = instead of ==. Or at the very least, I have to stop and think about what's going on. Yes, I get it, it lets you do something in one line of code, but I think it sacrifices readability and can make the intent ambiguous.
Someone on twitter asked me about that convention too - the best explanation I could find was that it's from C, where the compiler will warn you if you have a plain `if (foo = "bar")` in case you typo'd it. But putting double parens around it makes the compiler stop warning you, as you've intended the code to be that way by using a double parens. - http://stackoverflow.com/questions/3323257/double-parenthese... was the explanation I linked on twitter about it.
I couldn't trace the origins, but I believe it comes a long way from C. In K&R it was used quite freely in the form of
while ((c = getchar()) != EOF)
In this particular case it's necessary because of operators precedence, although I can't remember whether they used extra parens when it wasn't necessary.
In ruby most style guides, including github's[1], are forked from bbatsov's[2], which had the bullet point about using assignment return value.
But that just has to be several times slower than the boolean check with the array usage, enough that you wouldn't want this refactoring in a mega-loop, right?
steve at thoth in ~/tmp
$ cat benchmark.rb
require 'benchmark'
n = 500000
Benchmark.bm do |x|
x.report("==") do
n.times do
foo = "lol"
foo == "bar" || foo == "baz" || foo == :sed || foo == 5
end
end
x.report("include?") do
n.times do
foo = "lol"
["bar", "baz", :sed, 5].include?(foo)
end
end
end
steve at thoth in ~/tmp
$ ruby benchmark.rb
user system total real
== 0.370000 0.000000 0.370000 ( 0.367943)
include? 0.550000 0.000000 0.550000 ( 0.551897)
If it helps you/the code (readability, DRYness, ...) then go for it. Avoiding it on the basis of guessing it would make a difference is a slippery slope down premature optimization hell [1]. I'd say code away, and when/if you need to optimize, use a profiler. Likely you'll be surprised at what makes a difference and what doesn't really.
edit:
[1] Note that by guessing I'm referring to any optimizations that don't involve profiling your project, and that includes benchmarks of Ruby style, which are interesting to observe and can help you learn certain things about Ruby but are deceiving as far as understanding what actually makes a difference in your project. The sane way to optimize is to profile.
Just from futzing around with Benchmark, it looks like the array allocation is the expensive bit.
If you're not constantly reallocating the array you're checking against, it takes 1.5x more time to run. If you're allocating the array each time it's 3.5-4x more time.
Example #1 is actually a bad thing to do. This creates a new array every time you want to do the comparison. I've fixed multiple open-source libraries who abused this exact method, refactored them into the chained "and" statements, and seen a hefty performance increase as a result (MongoMapper and Haml come to mind)
Example #3 suffers from unbalanced returns and is less clear in its intent than the original (albeit difficult to read) example. Instead, I'd propose:
def my_method
@blah == foo && :foo_matches || :no_match
end
This eliminates the visual problem of adjacent semicolons, while still being clear and concise.
I'd argue that Example #4 is bad because it hides the expectation that nil should be returned in the negative case.
Object#try should work in plain 1.9 too, although I'm not sure I'd have reached for it in this case. This was more about showing the DRYing up a (visual) pattern across two lines into a different construct though, so perhaps I would've used try if it was just one line somewhere in my code.
That said, I didn't know try could take a block instead of a symbol, so thanks for sharing :-)
One thing: replacing
with doesn't look like an improvement to me. The first style is direct and readable, and lowers the barrier of entry to understand it. The second one requires to be familiar with the subtleties of Ruby, which adds difficulty when you're working with multiple languages daily or getting started with Ruby. I'm not saying you should avoid pushing Ruby to its best capabilities so that it's readable by any programmer, but in this case, winning 4 characters doesn't seem worth the compromise.