ruby - 我怎样才能让这个 Controller 方法不那么重？

Question

假设我有以下 Controller 和包含的方法。我觉得这个 Controller 方法(列表)太重了。我应该如何拆分？什么应该移到 View 层，什么应该移到模型层？

注意:返回格式(包含 :text、:leaf:、id、:cls 的散列)包含与 ProjectFile 数据库表中不同的字段，所以这让我质疑实际上应该将多少 Controller 方法移动到模型层。

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

Answer 1

我认为您可以将大部分逻辑放在您的模型 ProjectFile 或任何合适的模型名称中:

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 分解为更易于管理的方法。我在上面定义的方法很长并且会做很多事情(内聚性低)，因此将其分解将有助于克服这些缺点。

在您的 Controller 中，您可以这样调用它:

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

现在您的 Controller 更易于阅读，并且提取了业务逻辑。这遵循了“瘦 Controller ，胖模型”的口头禅。