开发者

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
0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜