Comment refactoriser ce code Ruby (controller)?

Question

Ceci est le code dans mon contrôleur de rapports, il semble juste si mauvais, quelqu'un peut-il me donner quelques suggestions sur la façon de ranger?

# 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 

Je cherche à utiliser une méthode comme

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

Answer 1

Ma pensée instantanée est: déplacer le code vers un modèle.

L'objectif devrait être "Thin Controllers", donc ils ne devraient pas contenir de logique métier. En second lieu, j'aime présenter mes lignes de rapport à mes vues en tant qu'objets OpenStruct(), qui me semble plus propre. Donc je considérerais déplacer cette logique d'accumulation dans (probablement) une méthode de classe sur le rapport et renvoyant un tableau de "ligne de rapport" OpenStructs et un seul totaux OpenStruct pour passer à ma vue.

Code Mon contrôleur deviendrait quelque chose comme ceci:

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

EDIT: (Un jour plus tard)

regardant que l'ajout chose d'accumulation de dans-un-tableau, je suis venu avec ceci:

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 

Regardez: un test! Cela semble fonctionner. Il n'y a pas de vérification des différences de taille entre les deux baies, donc il y a beaucoup de façons que ça puisse aller mal, mais le concept semble assez solide. Je pense essayer quelque chose de similaire qui me permettrait de prendre un jeu de résultats ActiveRecord et de l'accumuler dans un OpenStruct, ce que j'ai tendance à utiliser dans mes rapports ...

Notre nouvelle méthode Array pourrait réduire le code original à quelque chose comme ceci:

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 

... qui si le rapport # data_of_invoicing_and_delivery_report pourrait également calculer gross_profit permettrait de réduire encore plus loin:

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

complètement non testé, mais c'est un enfer d'une réduction de l'un ddition d'une méthode sur une ligne à Array et exécution d'une seule soustraction supplémentaire dans un modèle.

Answer 2

Créer un objet de sommation qui contient tous les champs, passer le tableau entier à @ sum.increment_sums (Report.data_of ...)