Как провести рефакторинг этого кода Ruby (контроллера)? - PullRequest
Новая
       5

Как провести рефакторинг этого кода Ruby (контроллера)?

0 голосов
/ 03 июня 2009

Это код в моем контроллере отчетов, он выглядит так плохо, кто-нибудь может дать мне несколько советов, как привести его в порядок? 

# 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 ]

5 голосов
/ 03 июня 2009

Моя мысль: переместить код в модель.

Целью должны быть «Тонкие контроллеры», чтобы они не содержали бизнес-логику.

Во-вторых, мне нравится представлять строки моего отчета в моих представлениях как объекты OpenStruct (), что мне кажется чище.

Таким образом, я бы рассмотрел возможность перемещения этой логики накопления в (наиболее вероятный) метод класса в Report и возвращения массива «строки отчета» OpenStructs и одного итогового значения OpenStruct для передачи в мой View.

Мой код контроллера станет примерно таким: 

@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, что я обычно использую в своих отчетах ...

Наш новый метод Array может уменьшить исходный код до чего-то вроде этого: 

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

... который, если Report # data_of_invoicing_and_delivery_report также может вычислить gross_profit, уменьшится еще больше до: 

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

Полностью не проверено, но это чертовски сокращение для добавления метода с одной строкой в ​​массив и выполнения одного дополнительного вычитания в модели.

2 голосов
/ 03 июня 2009

Создать объект суммирования, содержащий все эти поля, передать весь массив в @ sum.increment_sums (Report.data_of ...)

...