Rails 控制器变量混乱

Question

我有一个控制器，我觉得它的实例变量太多。

控制器从各个地方拉数据，感觉真的很草率。 我看过一些 Sandi Metz 的演讲、阅读书籍和其他研究，我想好好练习，但我不知道在这里做什么。

这种方法正在提取所有数据并将其发送到我的视图中，我能够让它工作，我只知道这不是一个好方法，我希望有人能指出我可以查看一些代码示例、文档、视频，或者帮助我了解如何实现更好的样式。

我在 SO 和 Google 上进行了搜索，但我发现大多数人都说要向视图发送哈希或 JSON，我想在开始之前知道这是否理想。 p>

Client、Project、Person、Role 控制器和模型具有非常相似的代码，我正在努力将其重构为更加 DRY。

例如，Client、Project、Person 和 Role 财务控制器具有与此几乎完全相同的控制器索引代码。 :(

如果有帮助，我很乐意添加更多代码！

这是project_financials_controller#index

它几乎是从视图中获取数据并从数据库中提取一堆数据并将其发送到视图。我目前只使用 index 方法，因为它应该只是一个“视图”，但现在我们可以添加过滤器，例如时间、不同的客户端等，所以我认为我需要以某种方式将其分解。

我确实有一个名为 financial_reports_nav 的模型，我可能会使用更多，甚至可以创建一个 financial_reports_controller 从适当的模型中提取数据，我什至不需要 4 个不同的控制器......

我完全愿意接受任何意见/批评！

def index
  # CPPR = Client, Project, Person, Role
  @financial_type = 'project'
  @financial_params = params

  # This pulls the timeframe from the view and figures out the dates requested. (eg. "Last Week")
  @timeframe = Financial.time_frame(@financial_params[:timeframe], current_company.timezone, params[:start_date], params[:end_date])

  # This grabs all the data required to recall this financial report view at a later time
  @financial_nav = FinancialReportNav.set_financial_type(@current_user.id,@financial_type, @start_date, @end_date)

  # Grab all active and inactive people for client
  @people = Person.active.all
  @deleted_people = Person.inactive.all

  # This sends over all the info needed to generate the financial reports
  @project_financial_populate = Financial.new(@financial_params, @financial_type).populate_project_financials(current_company.default_hourly_cost, current_company.billing_rate, @timeframe[:start_date],@timeframe[:end_date])

  # This just pulls all the data from the database that the @project_financial_populate just populated (Can't we just use that??)
  @financial_rows = ProjectFinancial.all.map { |p| [ p.project_id, p.billable_hours, p.revenue,p.real_rate, p.hourly_expense, p.labor_expense_total, p.salary_expense,  p.gross_profit, p.profit_margin, p.missing_hourly_expense, p.missing_billable_rate ] }

  # Using the same view for CPPR's
  # Clients has an items count, so we just stuff everything into the first array slot
  @items = [1]

  # If these are not null then they show an option to change the financial filter type.
  @filter_by_client = Client.find_by('id = ?', @financial_params[:filter_by_client])
  @filter_by_project = Project.find_by('id = ?', @financial_params[:filter_by_project])
  @filter_by_person = Person.find_by('id = ?', @financial_params[:filter_by_person])
  @filter_by_role = PersonRole.find_by('id = ?', @financial_params[:filter_by_role])

  # This pulls a list of CPPR's that have tracked time in the requested timeframe
  @project_list = Financial.project_list(@timeframe[:start_date], @timeframe[:end_date])
  @client_list = Financial.client_list(@timeframe[:start_date], @timeframe[:end_date])
  @people_list = Financial.people_list(@timeframe[:start_date], @timeframe[:end_date])
end

Answer 1

每当我注意到我至少有 3 个重复代码实例时，我总是倾向于将代码重构为 DRY，但我需要对新代码进行未来验证，以使其足够灵活以应对未来可能发生的变化；所有这些都考虑到了，但时间允许。

鉴于您已经使用了当前代码并告知了我的偏好，我会这样做：

1. 模型继承
2. 控制器继承
3. 共享模板

路线

config/routes.rb

resources :client_financial
resources :project_financial
resources :person_financial
resources :role_financial

型号

app/models/financial_record.rb

class FinancialRecord < ActiveRecord::Base # or ApplicationRecord if > Rails 5
  self.abstract_class = true

  # your shared "financials" model logic here
end

app/models/client_financial.rb

class ClientFinancial < FinancialRecord
  # override "financials" methods here if necessary
  # or, add new model specific methods / implementation
end

app/models/project_financial.rb

class ProjectFinancial < FinancialRecord
  # override "financials" methods here if necessary
  # or, add new model specific methods / implementation
end

app/models/person_financial.rb

class PersonFinancial < FinancialRecord
  # override "financials" methods here if necessary
  # or, add new model specific methods / implementation
end

app/models/role_financial.rb

class RoleFinancial < FinancialRecord
  # override "financials" methods here if necessary
  # or, add new model specific methods / implementation
end

控制器

app/controllers/financial_controller.rb

class FinancialController < ApplicationController
  before_action :set_instance_variables, only: :index

  protected

  def set_instance_variables
    # strips the last "Controller" substring and change to underscore: i.e. ProjectFinancialsController becomes project_financials
    @financial_type = controller_name[0..(-'Controller'.length - 1)].underscore

    # get the corresponding Model class
    model = @financial_type.camelcase.constantize
    # get the correspond Financial Model class
    financial_model = "#{@financial_type.camelcase}Financial".constantize

    @financial_params = params

    @timeframe = Financial.time_frame(@financial_params[:timeframe], current_company.timezone, params[:start_date], params[:end_date])

    # I dont know where you set @start_date and @end_date
    @financial_nav = FinancialReportNav.set_financial_type(@current_user.id,@financial_type, @start_date, @end_date)

    # renamed (or you can set this instance variable name dynamically)
    @records = model.active.all
    # renamed (or you can set this instance variable name dynamically)
    @deleted_records = model.inactive.all

    @financial_populate = Financial.new(@financial_params, @financial_type).populate_project_financials(current_company.default_hourly_cost, current_company.billing_rate, @timeframe[:start_date],@timeframe[:end_date])

    @financial_rows = financial_model.all.map { |p| [ p.project_id, p.billable_hours, p.revenue,p.real_rate, p.hourly_expense, p.labor_expense_total, p.salary_expense,  p.gross_profit, p.profit_margin, p.missing_hourly_expense, p.missing_billable_rate ] }

    @items = [1]

    @filter_by_client = Client.find_by('id = ?', @financial_params[:filter_by_client])
    @filter_by_project = Project.find_by('id = ?', @financial_params[:filter_by_project])
    @filter_by_person = Person.find_by('id = ?', @financial_params[:filter_by_person])
    @filter_by_role = PersonRole.find_by('id = ?', @financial_params[:filter_by_role])

    @project_list = Financial.project_list(@timeframe[:start_date], @timeframe[:end_date])
    @client_list = Financial.client_list(@timeframe[:start_date], @timeframe[:end_date])
    @people_list = Financial.people_list(@timeframe[:start_date], @timeframe[:end_date])
  end
end

app/controllers/client_financials_controller.rb

class ClientFinancialsController < FinancialController
  def index
    render template: 'financials/index'
  end
end

app/controllers/project_financials_controller.rb

class ProjectFinancialsController < FinancialController
  def index
    render template: 'financials/index'
  end
end

app/controllers/person_financials_controller.rb

class ProjectFinancialsController < FinancialController
  def index
    render template: 'financials/index'
  end
end

app/controllers/role_financials_controller.rb

class ProjectFinancialsController < FinancialController
  def index
    render template: 'financials/index'
  end
end

观看次数

app/views/financials/index.html.erb

<!-- YOUR SHARED "FINANCIALS" INDEX HTML HERE -->

附言这只是一个简单的重构。在不了解项目的更全面范围和未来计划的情况下，我只会做这个。话虽如此，我会考虑使用“多态”关联，然后只有一个路由端点（即resources :financials），然后传入一个参数过滤器，例如：params[:financial_type]，它直接已经映射了financial_type多态列名.