views:

1959

answers:

4

What is the recommended way of handling the following type of situations:

Supposing I have a model called Cart, that has a 1-1 relationship with the model Person and the same PK (the user's id).

In the index method of my cart_controller I want to check if a Cart exists for the current user. If I do Cart.find(the_user_id) and a cart doesn't exists a RecordNotFound exception gets raised.
I see two ways of solving this:

1. rescue from the exception

 begin
    @cart = Cart.find(the_user_id)
    #more code here
 rescue ActiveRecord::RecordNotFound
    #the cart is empty message
 end

2. use ActiveRecord#exists? method

 if Cart.exists?(the_user_id)
    @cart = Cart.find(the_user_id)
    #more code here
 else
    #the cart is empty message
 end

From my (limited) knowledge on exeption handling I know that it's not recommended to use exceptions this way, but is making an extra query every time worth it?

A: 

exists? would result in one more SQL statement, unless ActiveRecord guys optimized this out (I wouldn't count on that).

So I'd advise to use an exception, it's much cheaper than a SQL statement.

alamar
ActiveRecord has a pretty good query cache now. You really shouldn't use exceptions for control flow.
John Topley
Agreed, no exception necessary as this isn't really an exceptional situation, it most likely is going to happen a lot.
danengle
+12  A: 

You could try asking the user object for its cart. Let's say you have the user assigned to @user then if the user has a cart it would be @user.cart. If @user.cart is nil then they don't have one.

This assumes that you have the relationships between the models set up correctly.

Lee Irving
+1, This is the more correct method for rails.
workmad3
+1, ok. this sounds like the best way to go in this case. unfortunately my problem with #exists? vs rescue didn't get the argument sustained, good answer I was hoping for
andi
not sure what you mean by sustained argument. If you use cart = Cart.find(:id) then if its not found you will get an error which you have to trap using rescue. If you use cart = Cart.find_by_id(:id) then cart will be nil which you can test [email protected] will be nil or the cart which is very easy to test for. You can do something likeif @cart = @user.cart #do cart stuffelse # do no cart stuffend
Lee Irving
Yes, in this case this is definitely the best thing to do. But I was considering a general case, when I have to call a model directly (no associations) (eg find by id). Seems like I got the answer to that too, with find_by_id, which does not raise an execption.
andi
Actually in your case you would need to use Cart.find_by_user_id(user_id)
nasmorn
+6  A: 

Use find_by_id instead of find:

@cart = Cart.find_by_id(params[:id])

nil's if it does not exist so you can check "if @cart" in your controller/view as needed

Fer
+1 Interesting. Good to know. Thanks. I'm gonna go with @user.cart in this case though.
andi
John's answer is closer to your problem domain than mine, but think of if as an extra tool if the model does not belongs to another, (i.e is a Singleton or a root model)
Fer
+2  A: 

Why don't you do something like...

@cart = @user.cart || @user.cart.new

No worrying about exceptions or if/else statements. Then in your view you could have something like...

<% if @cart.empty? # or whatever method you use to determine 
     # if there is nothing in the cart...maybe .blank? is fine? 
%>
    <p>Your cart is empty</p>
<% else %>
    <!-- loop through objects in your cart -->
<% end %>
danengle
+1 yeah, @user.cart was the thing to do and not calling the Cart model directly. I accepted Lee Irving's answer though, because he was the first
andi