Making a fat controller in rails 3 skinny
I have this terribly large controller in my app. I'd really like to make it as skinny as possible. Below is some of the code, showing the types of things I'm currently doing.. I'm wondering what things I can move out of this?
A note - this is not my exact code, a lot of it is similar. Essentially every instance variable is used in the views - which is why I dont understand how to put the logic in the models? Can models return the values for instance variables?
def mine
#For Pusher
@push_ch = "#{current_user.company.id}"+"#{current_user.id}"+"#{current_user.profile.id}"
#Creating a limit for how many items to show on the page
@limit = 10
if params[:limit].to_i >= 10
@limit = @limit + params[:limit].to_i
end
#Setting page location
@ploc="mine"
@yourTeam = User.where(:company_id => current_user.company.id)
#Set the user from the param
if params[:user]
@selectedUser = @yourTeam.find_by_id(params[:user])
end
#Get all of the user tags
@tags = Tag.where(:user_id => current_user.id)
#Load the user's views
@views = View.where(:user_id => current_user.id)
if !params[:inbox]
#Hitting the DB just once for all the posts
@main_posts = Post.where(:company_id => current_user.company.id).includes(:status).includes(:views)
@main_posts.group_by(&:status).each do |status, posts|
if status.id == @status.id
if @posts_count == nil
@posts_count = posts
else
@posts_count = @posts_count + posts
end
elsif status.id == @status_act.id
if @posts_count == nil
@posts_count = posts
else
@posts_count = @posts_count + posts
end
end
end
if params[:status] == "All" || params[:status] == nil
@posts = Post.search(params[:search]).status_filter(params[:status]).user_filter(params[:user]).order(sort_column + " " + sort_direction).where(:company_id => current_user.company.id, :status_id => [@status.id, @status_act.id, @status_def.id, @status_dep.id, @status_up.id]).limit(@limit).includes(:views)
else
@posts = Post.search(params[:search]).status_filter(params[:status]).user_filter(params[:user]).order(sort_column + " " + sort_direction).where(:company_id => current_user.company.id).limit(@limit).includes(:views)
end
elsif params[:inbox] == "sent"
@yourcompanylist = User.where(:company_id => current_user.company.id).select(:id).map(&:id)
@yourcompany = []
@yourcompanylist.each do |user|
if user != current_user.id
@yourcompany=@yourcompany.concat([user])
end
end
if params[:t]=="all"
@posts = Post.search(params[:search]).status_filter(params[:status]).user_filter(params[:user]).tag_filter(params[:tag], current_user).order(sort_column + " " + sort_direction).where(:user_id => current_user.id).includes(:views, :tags).limit(@limit)
elsif params[:status]!="complete"
@posts = Post.search(params[:search]).status_filter(params[:status]).user_filter(params[:user]).tag_filter(params[:tag], current_user).order(sort_column + " " + sort_direction).where(:user_id => current_user.id).includes(:views, :tags).limit(@limit)
elsif params[:status]!=nil
@posts = Post.search(params[:search]).status_filter(params[:status]).user_filter(params[:user]).tag_filter(params[:tag], current_user).order(sort_column + " " + sort_direction).where(:user_id => current_user.id).includes(:views, :tags).limit(@limit)
end
end
respond_to do |format|
format.html # index.html.erb
format.js # index.html.erb
format.xml { render :xml => @posts }
end
end
开发者_Go百科
You can start by moving logic into the model...
A line like this screams of feature envy:
@push_ch = "#{current_user.company.id}"+"#{current_user.id}"+"#{current_user.profile.id}"
I would recommend moving it into the model:
#user.rb
def to_pusher_identity
"#{self.company_id}#{self.id}#{self.profile_id}"
end
And then in your controller
@push_ch = current_user.to_pusher_identity
At this point you could even move this into a before_filter
.
before_filter :supports_pusher, :only => :mine
Another thing you can do is create richer associations, so you can express:
@tags = Tag.where(:user_id => current_user.id)
as
@tags = current_user.tags
Another example would be for main posts, instead of
Post.where(:company_id => current_user.company.id).includes(:status).includes(:views)
you would go through the associations:
current_user.company.posts.includes(:status).includes(:views)
When I'm drying out a controller/action I try to identify what code could be (should be?) offloaded into the model or even a new module. I don't know enough about your application to really point to where these opportunities might lie, but that's where I'd start.
Few quick ideas:
Consider using respond_to/respond_with. This controller action can be splitted up to two separate ones - one for displaying @main_posts, another for params[:inbox] == "sent". The duplicate code can be removed using before_filters.
Also, a couple of gem suggestions:
- use kaminari or will_paginate for pagination
- meta_search for search and sorting
精彩评论