51224bdd17878b3b19e8987e9bb336a2

Hi,
In following code I am listing the users on index page with age & salary column.
Is there any better way to do the following thing.
-Database should not get hit more than once.

Thanks
DG

Users controller

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
class UsersController < ApplicationController

  def index
    @users = User.find(:all,:order => "age DESC")
    session[:descending] = @users
  end

  def ascending_age
    ord = session[:descending]
    @users = ord.sort{|x,y| x.age <=> y.age}
    render :action=>"index"
  end

  def ascending_salary
    ord = session[:descending]
    @users = ord.sort{|x,y| x.salary <=> y.salary}
    render :action=>"index"
  end
end

Refactorings

No refactoring yet !

1e8f141e7857d397d8020ed3b759e88a

Maciej Piechotka

August 6, 2008, August 06, 2008 13:54, permalink

No rating. Login to rate!

Why you don't use JavaScript?
Just sort the data on user page without hitting the database or even server.

1e8f141e7857d397d8020ed3b759e88a

Maciej Piechotka

August 6, 2008, August 06, 2008 14:00, permalink

No rating. Login to rate!

Why you don't use JavaScript? You will have it wothout even hitting server.
You can of course fallback to hitting server.

Do not use session for storing objects. You ask for problems (mostly performance problems).

Users controller

1
2
3
4
5
6
7
8
class UsersController < ApplicationController

  def index
    by = %w{age salary}.include? params['by'] ? params['by'] : 'age'
    dir = %w{ASC DESC}.include? params['dir'] ? params['dir'] : 'DESC'
    @users = User.find(:all, :order => "#{by} #{dir}")
  end
end
5071c5b861341c0dcfcf6ac86327701f

Tien Dung

August 6, 2008, August 06, 2008 14:07, permalink

No rating. Login to rate!

Hi DG,

1) We should not put model data (User model) in session because:
- Session is used to hold only the TEMPORARY data between a specific user and our web app.
- Session data should be kept as small as possible. For most of the case the user_id is enough.
- Model data is PERSISTENT data and should be consistent with all views (from different users).
- If we keep model data in session, each user will have a different copy of model data. It invalid the consistency.

2) We should not put model-related code in controller. We can move them to model.rb file to keep controller slim :)
Another benefit of putting logic code in model is that you can test code more easier.
You can "script/console" and try model's functions or write spec tests for them.

3) Don't afraid database queries will slow down the app. Finding & sorting are the jobs of database system.

Base on 3 points above I refactor your code as below.
I use param variable params[:order_by] to guide the controller to sort @users by age_ascending, age_descending and salary_ascending.
Urls will look like:
/users?
=> show all users ordered by age DESC

/users?order_by=age
=> show all user ordered by age ASC

/users?order_by=salary
=> show all user ordered by salary ASC

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
class UsersController < ApplicationController
  def index
    @users = find_all_and_order_by(params[:order_by])
  end
end


# user.rb
class User < ActiveRecord::Base
  named_scope :order_by_age_descending,   :order => "age DESC"
  named_scope :order_by_age_ascending,    :order => "age ASC"
  named_scope :order_by_salary_ascending, :order => "salary ASC"

  def self.find_all_and_order_by( field_name )
    case field_name
    when "age"
      User.order_by_age_ascending.all
    when "salary"
      User.order_by_salary_ascending.all
    default
      User.order_by_age_descending.all
    end
  end
end

# Now we can "script/console" and try User.find_all_and_order_by("age")
51224bdd17878b3b19e8987e9bb336a2

DG

August 7, 2008, August 07, 2008 05:08, permalink

No rating. Login to rate!

Hi Maciej Piechotka
Thanks for refactoring.
by the way,I tried to implement Javascript sort but didn't get any success. Could someone send me some tutorials or links so that I will get detail idea about how to do Javascript sorting.

Thanks
DG

6c3603a7bde5dc436753c53e56cb67e1

Corey Grusden

August 12, 2008, August 12, 2008 19:30, permalink

No rating. Login to rate!

If you don't want to hit the database more than once, use memcached for returned results. Also, with everyone else's recommendation, do NOT put result sets into your session. Focus on adding features and business value instead of pre-mature optimization that more than likely doesn't matter at this point in your application's life.

Your refactoring





Format Copy from initial code

or Cancel