class MyaccountController < ApplicationController
def account_details
@users = User.find(:all,:conditions=>"age > #{current_user.age}")
end
def show_email_box
@users = User.find(:all,:conditions=>"age > #{current_user.age}")
end
def show_mobile_box
@users = User.find(:all,:conditions=>"age > #{current_user.age}")
end
end
Refactorings
No refactoring yet !
rafeco
July 31, 2008, July 31, 2008 13:31, permalink
You could create a before_filter to DRY up the code a bit, as shown in the refactoring. Trying to cache the users so you only have to hit the database once (for some unspecified period of time) seems fraught with risk.
class MyaccountController < ApplicationController
before_filter load_users, :only => [ :account_details, :show_email_box, :show_mobile_box ]
def account_details
end
def show_email_box
end
def show_mobile_box
end
protected
def load_users
@users = User.find(:all,:conditions=>"age > #{current_user.age}")
end
end
Fabien Jakimowicz
July 31, 2008, July 31, 2008 13:33, permalink
You can use a before filter to avoid code duplication.
But keeping 'dynamic' data between queries is not always a good idea : data can change between 2 queries and you can have inconsistent data.
Anyway, you can use session to store data between queries. Here are 2 solutions.
You can even remove action definition if there are empty, but you will loose code clarity
class MyaccountController < ApplicationController
before_filter :load_users, :only => [ 'account_details', 'show_email_box', 'show_mobile_box' ]
def account_details
end
def show_email_box
end
def show_mobile_box
end
private
def load_users
@users = User.find(:all,:conditions=>"age > #{current_user.age}")
end
end
class MyaccountController < ApplicationController
before_filter :load_users, :only => [ 'account_details', 'show_email_box', 'show_mobile_box' ]
def account_details
end
def show_email_box
end
def show_mobile_box
end
private
def load_users
session[:users] ||= User.find(:all,:conditions=>"age > #{current_user.age}")
@users = session[:users]
end
end
danielharan
July 31, 2008, July 31, 2008 13:51, permalink
Persistent storage by adding it to the session smells. Take Fabien's first solution to avoid code duplication. If you really are facing performance issues, then look in to caching the front-end (assuming you're keeping the list of @users in a partial).
Jeremy Weiskotten
July 31, 2008, July 31, 2008 14:21, permalink
If you're using Rails 2.1, I also like using scopes. This would be overkill if you don't need the logic in more than one controller, but it's an expressive way to encapsulate commonly-used queries... any they can be combined with each other, such as User.older_than(me).male.with_hair('brown')
class User
named_scope :older_than, lambda { |u| { :conditions => ['age > u', u.age] } }
end
@users = User.older_than current_user
Adam
August 11, 2008, August 11, 2008 18:32, permalink
I'm not sure exactly what you are trying to do with all those actions, but from what has been given it feels like it might be appropriate to treat them as alternate content types rather than separate actions.
class UsersController < ApplicationController
# /users => app/views/users/users.html.erb
# /users.email => app/views/users/users.email.erb
# /users.mobile => app/views/users/users.mobile.erb
def index
@users = User.older_than(current_user.age)
end
end
Mime::Type.register_alias "text/html", :mobile Mime::Type.register_alias "text/html", :email
Hi Following is the code snippet
- I am just fetching users who have age more then current user age, I am doing same thing in 3 actions.
- Is there any way to keep @users variable persistent for 3 actions. or for full controller. and also my database should not get hit more than once.
- I also tried using class inheritable attributes but No success.
Thanks
DG