Geek asks and answers

How can I make this controller method less heavy?

Let's say I have the following controller and containing method. I feel this controller method (list) is too heavy. How do I share this? What needs to be transferred to the presentation level and what needs to be transferred to the model level?

Note. The return format (a hash containing: text ,: leaf :, id ,: cls) contains different fields from the ProjectFile database table, so this asks me how much of this controller method needs to be transferred to the model layer.

class ProjectFileController < ApplicationController
  before_filter :require_user

  def list
    @project_id = params[:project_id]
    @folder_id = params[:folder_id]

    current_user = UserSession.find
    @user_id = current_user && current_user.record.id

    node_list = []

    # If no project id was specified, return a list of all projects.
    if @project_id == nil and @folder_id == nil
      # Get a list of projects for the current user.
      projects = Project.find_all_by_user_id(@user_id)

      # Add each project to the node list.
      projects.each do |project|
        node_list << {
          :text => project.name,
          :leaf => false,
          :id => project.id.to_s + '|0',
          :cls => 'project',
          :draggable => false
        }
      end
    else
      # If a project id was specfied, but no file id was also specified, return a
      # list of all top-level folders for the given project.
      if @project_id != nil and @folder_id == nil
        # Look for top-level folders for the project. 
        @folder_id = 0
      end

      directory_list = []
      file_list = []

      known_file_extensions = ['rb', 'erb', 'rhtml', 'php', 'py', 'css', 'html', 'txt', 'js', 'bmp', 'gif', 'h', 'jpg', 'mov', 'mp3', 'pdf', 'png', 'psd', 'svg', 'wav', 'xsl']

      # Get a list of files by project and parent directory.
      project_files = ProjectFile.find_all_by_project_id(@project_id,
                                                         :conditions => "ancestry like '%#{@folder_id}'",
                                                         :order => 'name')

      project_files.each do |project_file|
        file_extension = File.extname(project_file.name).gsub('.', '')

        if known_file_extensions.include? file_extension
          css_class_name = file_extension
        else
          css_class_name = 'unknown'
        end

        # Determine whether this is a file or directory.
        if project_file.is_directory
          directory_list << {
            :text => project_file.name,
            :leaf => false,
            :id => @project_id + '|' + project_file.id.to_s,
            :cls => css_class_name
          }
        else
          file_list << {
            :text => project_file.name,
            :leaf => true,
            :id => @project_id + '|' + project_file.id.to_s,
            :cls => css_class_name
          }
        end
      end

      node_list = directory_list | file_list
    end

    render :json => node_list
  end
end
+3
source share
3 answers

I think you could put most of this logic into your model, ProjectFileor something else, corresponding to the model name:

ProjectFile < ActiveRecord::Base
  def node_list(project_id, folder_id, user_id)
    node_list = []

    # If no project id was specified, return a list of all projects.
    if project_id == nil and folder_id == nil
    # Get a list of projects for the current user.
    projects = Project.find_all_by_user_id(user_id)

      # Add each project to the node list.
      projects.each do |project|
        node_list << {
          :text => project.name,
          :leaf => false,
          :id => project.id.to_s + '|0',
          :cls => 'project',
          :draggable => false
        }
      end
    else
      # If a project id was specfied, but no file id was also specified, return a
      # list of all top-level folders for the given project.
      if project_id != nil and folder_id == nil
        # Look for top-level folders for the project. 
        folder_id = 0
      end

      directory_list = []
      file_list = []

      known_file_extensions = ['rb', 'erb', 'rhtml', 'php', 'py', 'css', 'html', 'txt', 'js', 'bmp', 'gif', 'h', 'jpg', 'mov', 'mp3', 'pdf', 'png', 'psd', 'svg', 'wav', 'xsl']

      # Get a list of files by project and parent directory.
      project_files = ProjectFile.find_all_by_project_id(project_id,
                                                     :conditions => "ancestry like '%#{folder_id}'",
                                                     :order => 'name')

      project_files.each do |project_file|
        file_extension = File.extname(project_file.name).gsub('.', '')

        if known_file_extensions.include? file_extension
          css_class_name = file_extension
        else
          css_class_name = 'unknown'
        end

        # Determine whether this is a file or directory.
        if project_file.is_directory
          directory_list << {
            :text => project_file.name,
            :leaf => false,
            :id => project_id + '|' + project_file.id.to_s,
            :cls => css_class_name
        }
        else
          file_list << {
            :text => project_file.name,
            :leaf => true,
            :id => project_id + '|' + project_file.id.to_s,
            :cls => css_class_name
          }
        end
      end

      node_list = directory_list | file_list
    end
end

node_list . , , ( ), .

:

class ProjectFileController < ApplicationController
  before_filter :require_user

  def list
    @project_id = params[:project_id]
    @folder_id = params[:folder_id]

    current_user = UserSession.find
    @user_id = current_user && current_user.record.id

    nodes = node_list(@project_id, @folder_id, @user_id)
    render :json => nodes
  end
end

-. " , ".

+3

,

has_many => :projects

,

current_user.projects

:

projects = Project.find_all_by_user_id(@user_id)

, user_id.

node_list . :

def node_list(project_id, folder_id=0)
  if @project_id == nil
    list = self.projects.map do |project|
      {
        :text => project.name,
        :leaf => false,
        :id => project.id.to_s + '|0',
        :cls => 'project',
        :draggable => false
      }
    end
  else
    ... # the rest of your code here, etc
  end
  return list
end

, , _ ​​ , folder_id=0 def node_list(project_id, folder_id=0) .

:

def list
  @current_user = UserSession.find
  render :json => @current_user.node_list(params[:project_id], params[:folder_id])
end

_list file_list, if-else :

{
  :text => project_file.name,
  :leaf => project_file.is_directory,
  :id => @project_id + '|' + project_file.id.to_s,
  :cls => css_class_name
}

, .

+1

@Chad,

, , .

, , ruby-, " ".

    • ,
    • () "" .
    • , "" ( ), Rails . json, .
    • .
    • ", 10 , ".

    • .
    • .
    • , " " , if then else.
    • .
    • . , , , .
    • 1 2 . 3 . 3 .
    • , .
    • , . (has_many: , , ).
    • .
    • .
    • , , , , Enumerables. !

!:)

+1

More articles:


All Articles