2
\$\begingroup\$

I have Article and Category, with a has_and_belongs_to_many relation between the two.

I want to use Article.in_categories(category_ids) to get any Article that is in any of the category_ids passed, and I want to handle the possibility of category_ids being nil in a way that all Article items are returned. The category_ids value is coming from a form, and I want the user to be able to ignore the Category selection part of the form if they do not want to limit their selection by that criteria.

I currently have this in place on the Article model:

def self.in_categories(category_ids = nil)
  category_ids.blank? ? all : includes(:categories).where(categories: { id: category_ids })
end

If I do not have the check for category_ids being nil, then it will only return the Article items that are not in any Category.

I do not want to manually set category_ids = [1,2,3] (for example) in the method call, because a Category can be added/removed.

Is there a better way of handling this than the way I already do?

\$\endgroup\$
  • 1
    \$\begingroup\$ Not quite an answer, so I'll comment instead: I'd suggest handling possible nil values elsewhere, and simply not call in_categories unless you have some IDs and it makes sense to do so. Right now, in_categories is basically a sometimes-alias for all, which smells weird to me. I'd expect it always filter the article records somehow, given its name. \$\endgroup\$ – Flambino Apr 5 '14 at 19:28
3
\$\begingroup\$

Generally you'll want to make sure this always gets passed an array so I'd choose to set the argument to an empty array rather than nil. Since in the successful case the method receives an array, you should treat it as such. If you ever expect to get a nil passed in (like you might from params), you can also catch if it is blank or empty to handle all cases safely.

If you get rid of the ternary, you can simply put a guard as the first line and that clarifies the method a lot more. You'll see the use of guards throughout the Rails source itself.

def self.in_categories(category_ids=[])
  return all if category_ids.blank? || category_ids.empty?
  includes(:categories).where(categories: { id: category_ids })
end

Alternatively you could always cast the parameter to an array and only handle that case.

def self.in_categories(category_ids=nil)
  category_ids = category_ids.to_a
  return all if category_ids.empty?
  includes(:categories).where(categories: { id: category_ids })
end

I've run into this "undocumented feature" on several occasions with Rails and it trips me up every time. Hopefully someone has a better solution so that Rails, but it is also good to just be explicit on the results if you can't consolidate.

Looks like there are quite a few discussions about this https://github.com/rails/arel/pull/245

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.