When chaining several queries together in rails, how does one handle exceptions well?

Question

In several of my controllers, I have code looking something like this:

@user
  .cars.find_by_id(params[:car_id])
  .try(:wheels).find_by_id(params[:wheel_id])
  .try(:screews).where(id: ids)

This can of course raise the exception: NoMethodError: undefined method `where' for nil:NilClass. I would like to be able to give my user more information about what went wrong, in this case, that the user had no cars with the specified id.

Changing find_by_id to find would cause the above code to raise an ActiveRecord::RecordNotFound exception, which I could handle, but in that case, how do I find out if the cars or the wheels were missing? The RecordNotFound exception doesn't really contain that much information. Is there any good way to handle this type of thing well? Preferably, something that is general enough that I could easily reuse the code in my other controllers.

Answer 1

I would say that your code smells and you should refactor it.

• Each unit should have only limited knowledge about other units: only units "closely" related to the current unit.
• Each unit should only talk to its friends; don't talk to strangers.
• Only talk to your immediate friends.

http://en.wikipedia.org/wiki/Law_of_Demeter

I'm not saying that you need to worship the Law of Demeter (DHH has a pretty good post about this) or other CS concepts but rather that catching the exceptions in this case would not make the situation better but is like building a safety net around a tower with a spotty foundation so that the inhabitants can jump out the window when it crumbles.

Lets go through it row by row:

@user
  .cars.find_by_id(params[:car_id])
  .try(:wheels).find_by_id(params[:wheel_id])
  .try(:screews).where(id: ids)

.find_by_id is a so called dynamic finder method which relies on method missing, they where extracted from the core in Rails 4. Use .find instead, it is forward compatible and does not have the overhead of the method missing metamagic.

We don't need to scope our query from @user since we already have a unique id for cars.

Car.find(params[:car_id])
  .try(:wheels).find_by_id(params[:wheel_id])
  .try(:screews).where(id: ids)

If we want to fetch both the Car and the Wheel we should be using a join query to make an effective SQL query.

Car.joins(:wheels)
   .where(id: params[:wheel_id])
   .find(params[:car_id])

But if what we really need are the screws we can do this instead:

# Doing a JOIN to cars requires a 
# has_many :cars, through: :wheels relationsship
@screws = Screw.where(id: ids).joins(:cars, :wheels)

So to reiterate:

• Do not scope queries through the parent when you have a unique ID - you are only causing a N+1 query.
• Use joins to create more efficient queries.
• Use exceptions to handle EXCEPTIONAL cases, not normal program flow.

Answer 2

The ActiveRecord::RecordNotFound exception has a message i.e. Couldn't find User with id=200 so you can use it to tell you which model wasn't found.

Just rescue that exception and use the message to show the information to the user. You could search the model name in the message and show a custom prompt if you want to.