代码之家 › 专栏 › 技术社区 › Shuoling Liu

如何重构这个Ruby(控制器)代码?

automated-refactoring refactoring ruby ruby-on-rails

Shuoling Liu · 技术社区 · 15 年前

这是我的报表控制器中的代码,它看起来很糟糕,有人能给我一些关于如何整理它的建议吗?

# app\controller\reports_controller.rb

 @report_lines  = []
   @sum_wp, @sum_projcted_wp, @sum_il, @sum_projcted_il, @sum_li,@sum_gross_profit ,@sum_opportunities = [0,0,0,0,0,0,0]    
 date = @start_date

 num_of_months.times do
    wp,projected_wp, invoice_line,projected_il,line_item, opp = Report.data_of_invoicing_and_delivery_report(@part_or_service,date)
    @sum_wp += wp
    @sum_projcted_wp +=projected_wp
    @sum_il=invoice_line
    @sum_projcted_il +=projected_il
    @sum_li += line_item
    gross_profit = invoice_line - line_item
    @sum_gross_profit += gross_profit
    @sum_opportunities += opp
    @report_lines << [date.strftime("%m/%Y"),wp,projected_wp ,invoice_line,projected_il,line_item,gross_profit,opp]
    date = date.next_month
 end

我想用一些方法

@sum_a,@sum_b,@sum_c += [1,2,3]

2 回复 | 直到 15 年前

Mike Woodhouse 15 年前

我立刻想到的是:将代码移动到模型中。

目标应该是“瘦控制器”,因此它们不应该包含业务逻辑。

第二,我喜欢将我的报告行作为openstruct()对象呈现给我的视图,这对我来说更清晰。

因此,我考虑将这个累积逻辑转移到(最有可能的)报表上的类方法中,并返回一个“报告行”openstruct数组和一个totals openstruct传递给我的视图。

我的控制器代码会变成这样:

@report_lines, @report_totals = Report.summarised_data_of_inv_and_dlvry_rpt(@part_or_service, @start_date, num_of_months)

编辑:(一天后)

看着把积累添加到数组中,我想到了:

require 'test/unit'

class Array
  def add_corresponding(other)
    each_index { |i| self[i] += other[i] }
  end
end

class TestProblem < Test::Unit::TestCase
  def test_add_corresponding
    a = [1,2,3,4,5]
    assert_equal [3,5,8,11,16], a.add_corresponding([2,3,5,7,11])
    assert_equal [2,3,6,8,10], a.add_corresponding([-1,-2,-2,-3,-6])
  end
end

看,考试!它似乎工作正常。没有检查两个数组之间的大小差异,因此有很多方法可能出错,但这个概念似乎足够合理。我正在考虑尝试类似的方法,让我获取一个ActiveRecord结果集并将其累积到一个Openstruct中,这是我在报告中经常使用的方法…

我们的新数组方法可能会将原始代码减少到如下所示:

totals = [0,0,0,0,0,0,0]    
date = @start_date

num_of_months.times do
  wp, projected_wp, invoice_line, projected_il, line_item, opp = Report.data_of_invoicing_and_delivery_report(@part_or_service,date)
  totals.add_corresponding [wp, projected_wp, invoice_line, projected_il, line_item, opp, invoice_line - line_item]
  @report_lines << [date.strftime("%m/%Y"),wp,projected_wp ,invoice_line,projected_il,line_item,gross_profit,opp]
  date = date.next_month
end

@sum_wp, @sum_projcted_wp, @sum_il, @sum_projcted_il, @sum_li, @sum_opportunities, @sum_gross_profit = totals

…如果报告“发票”和“交货”报告的数据也可以计算 gross_profit 会进一步减少到:

num_of_months.times do
  totals.add_corresponding(Report.data_of_invoicing_and_delivery_report(@part_or_service,date))
end

完全未经测试,但这对于向数组中添加一行方法和在模型中执行单个额外减法来说是一个极大的减少。

Kevin Peterson 15 年前

创建一个包含所有这些字段的求和对象,将整个数组传递给@sum.increment_sums(report.data_of…)