So I ran into a bit of code that looked like this:
def my_method
list.each do |item|
return_value = item.some_property
return return_value if return_value
end
nil
end
And I thought, boy, is this lame or what? [1]
There are a couple of smells here. When you see a word (return_value
) three times in
the same function, you know it's a smell. So let's first just try to minimize this repetition:
def my_method
list.each { |item| return_value = item.some_property and return return_value }
nil
end
Feels better already, doesn't it? But another smell remains, and that's having to explicitly return
nil
when there's no match. A tool better suited for the task is actually the method find
, with
some help from lazy evaluation and a good knowledge of operator precedence in ruby:
def my_method
item = list.find { |item| item.some_property } and item.some_property
end
That is perfect, right? A one-liner. But we can top that, of course. some_property
is still being invoked
in two places, and that just feels wrong. Here's a clever solution that's very atomic using our favourite
pal, inject
:
def my_method
list.inject(nil) { |acc, item| acc || item.some_property }
end
I know, it's pure beauty. But if you're the kind of person who likes to muck around with the built-ins
(no problem with that), what you really want, is find
and map
together. a mapfind
, if you will (or
selectdetect
for the smalltalk-jargon-inclined):
module Enumerable
def mapfind(not_found = nil, &block)
(found = find(&block)) ? block[found] : not_found
end
end
And then it's as simple as:
def my_method
list.mapfind { |item| item.some_property }
end
and if you're on ruby 1.9 or using active_support in 1.8, shorten it even more:
def my_method
list.mapfind &:some_property
end
[1]: Of course, it could be lamer:
def my_method()
for item in list
return_value = item.some_property()
if (return_value)
return return_value
end
end
return nil
end