Skip to content

Instantly share code, notes, and snippets.

@kch
Created May 14, 2009 06:39
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save kch/111538 to your computer and use it in GitHub Desktop.
Save kch/111538 to your computer and use it in GitHub Desktop.
How to not waste lines and save kittens

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment