29

I have the following code:

def maturities
  InfoItem.find_all_by_work_order(self.work_order).map(&:maturity)
end

I was thinking about changing it to:

def maturities
  InfoItem.where(work_order: self.work_order).map(&:maturity)
end

Would there be any advantage to this? It seems like .where is more common than find_all_by nowadays.

ardavis
  • 9,842
  • 12
  • 58
  • 112
  • I'm in the process of upgrading an app from Rails 4.0.3 to 4.1.0 and my code that used `find_all_by` no longer works (`NoMethodError`). I don't see anything in the release notes that would affect it. I'll have to switch to `where`. Had I used `where` from the beginning, my code would have been less prone to such errors. There's [a comment below](http://stackoverflow.com/questions/11232971/rails-find-all-by-vs-where#comment14759921_11233522) mentioning that `find_all_by_*` would be deprecated in Rails 4. Still, this came as a surprise to me. Where is the removal of this method documented? – Dennis Apr 11 '14 at 20:25
  • I found where it's documented. In the 4.1 release notes: "Removed activerecord-deprecated_finders as a dependency. Please see the gem README for more info." – Dennis Apr 11 '14 at 20:53
  • I would also suggest using `pluck` instead of `map` in this type of situation. `InfoItem.where(work_order: self.work_order).pluck(:maturity)` – jurassic Sep 07 '14 at 19:48

2 Answers2

28

My opinion is that using .where is a better approach.

When you use attribute based finders, you are going to have to tunnel through a method missing call and eventually define a class method, via class_eval, that returns your result. This is extra processing that you may not need to do.

Also, stringing together: find_by_this_and_this_and_this_and_this... can get ugly.

See how rails accomplishes attribute based finders here

Method missing from module DynamicMatchers on github:

def method_missing(name, *arguments, &block)
  match = Method.match(self, name)

  if match && match.valid?
    match.define
    send(name, *arguments, &block)
  else
    super
  end
end
Kyle
  • 21,978
  • 2
  • 60
  • 61
  • Thank you for putting time and thought into this, I appreciate your answer. I'm going to allow some more time for other suggestions before I mark one as "my answer". Thank you sir. – ardavis Jun 27 '12 at 19:26
  • 1
    You're welcome. This is only my opinion and may very well not be the best explanation. Great question! – Kyle Jun 27 '12 at 19:27
  • 8
    `find_all_by_*` methods are going to be deprecated in Rails 4. It's best if you use a `where` here. – Ryan Bigg Jun 27 '12 at 21:03
  • Thanks @RyanBigg I appreciate your response. – ardavis Jun 27 '12 at 22:14
2

I think the main advantage is being able to add additional criteria to where, find_all_by is limited to the field of the dynamic selector. If you only have one condition you are searching by then I think it is a wash, but when you start adding 3 or 4, dynamic finders can be ugly. Hashes are nice to look at, and you could pass a hash of conditions as a parameter if needed. Dynamic finders are cool, but I think where scales in a cleaner way and is more readable.

OpenCoderX
  • 6,180
  • 7
  • 34
  • 61
  • But I can successfully do something like: `InfoItem.find_all_by_work_order_and_description(self.work_order, "bla bla bla")` – ardavis Jun 27 '12 at 19:13
  • I don't mean that you can't add more to a dynamic finder, but passing a hash as a parameter to where is cleaner. IMO – OpenCoderX Jun 27 '12 at 19:19
  • 2
    AFAIK .where is the 'Rails 3 way' of doing it, not to mention it is a lot cleaner and more flexible. – Felipe Lima Jun 27 '12 at 19:23
  • I agree to that completely, I was curious on the thoughts of the community, so far, I like what I'm seeing. – ardavis Jun 27 '12 at 19:25