Best practice question

Hello guys!

Question:

Views should be “dumb” and models should deal with db queries…

but what if you have a situation like so

class User < ActiveRecord::Base

def has_relationship_with?( other_user )
!relationships.find_by_other_user_id( other_user.id ).nil?
end
end

and then in your view:

<%= “Display this message” if @user.has_relationship_with?
(@other_user) %>

Is it cool to call model methods which, in turn, query the db within
the view or is there a better way of implementing this?

Thanks

ASH

On Jul 27, 2009, at 12:26 PM, Ashley wrote:

def has_relationship_with?( other_user )
the view or is there a better way of implementing this?

Thanks

ASH

This should be fine, but you might want to think about whether you
need to “eager load” the relationships assosciation for performance.

The benefit is that your model contains the code and you can
potentially refactor without touching your view.

-Rob

Rob B. http://agileconsultingllc.com
[email protected]

You might want to memoize the results of that function to prevent
unnecessary hits to the database during the same request.

On Mon, Jul 27, 2009 at 2:21 PM, Rob B.
[email protected]wrote:

class User < ActiveRecord::Base

The benefit is that your model contains the code and you can

Jim
http://www.thepeoplesfeed.com/contribute

Hi Guys!

Thanks for both of your input.

I’ve opted for a helper method that looks something like:

module UsersHelper

def has_relationship_with?( other_user )
var_name = “@has_relationship_with_#{other_user.id}”
instance_variable_get(var_name) || instance_variable_set(var_name,
current_user.has_relationship_with?( other_user )
end

end

Loading all of the relationships was a little unnecessary in this case
but making the same db query more than once per request was too so
I’ve stuck the value in an instance variable

you reckon that’s ok?

On Jul 27, 2009, at 3:21 PM, Ashley wrote:

instance_variable_get(var_name) || instance_variable_set(var_name,
current_user.has_relationship_with?( other_user )
end

end

Loading all of the relationships was a little unnecessary in this case
but making the same db query more than once per request was too so
I’ve stuck the value in an instance variable

you reckon that’s ok?

Uh, a helper on the view? In this case, I’d say no.

I’m assuming that you really want to have several:

<%= “Interesting stuff” if @user.has_relationship_with?(@other_user) %>

<%= “Other stuff” if @user.has_relationship_with?(@other_user) %>

<%= “Boring stuff” if @user.has_relationship_with?(@other_user) %>

pieces in your view and you don’t want 3 calls, only 1. Is that right?

If so, then your original solution may have done it. (ActiveRecord is
pretty smart most times.)

class User < ActiveRecord::Base
def has_relationship_with?( other_user )
relationships.find_by_other_user_id( other_user.id )
end
end

Once the relationships association is loaded, it might just get later
results from what’s already in memory. Take a look at your log/
development.log and see if you find entries like: (but for
Relationship, of course)

School Load (0.000439) SELECT * FROM “schools” WHERE
(“schools”.“id” = 40882)
CACHE (0.000000) SELECT * FROM “schools” WHERE (“schools”.“id” =
40882)
CACHE (0.000000) SELECT * FROM “schools” WHERE (“schools”.“id” =
40882)

These are from a view helper (which in my case is actually an
ActiveRecord model backed by a real database view rather than a
table) that needs to find a record for the School model.

One thing to note is that you never have to say: !object.nil?
Just say: object
Since ruby only considers false and nil to be “false” in a boolean
context, it is idiomatic for predicates (i.e., methods that provide a
yes/no answer) to return false/true or nil/object. Since
find_by_other_user_id is already going to give you the Relationship
object when it exists and nil otherwise, you don’t need to turn that
into false/true.

-Rob

Question:
end
Thanks

Rob B. http://agileconsultingllc.com
[email protected]

Jimhttp://www.thepeoplesfeed.com/contribute

Rob B. http://agileconsultingllc.com
[email protected]
+1 513-295-4739
Skype: rob.biedenharn