重构大型控制器方法到模型

Question

我有一个rails应用程序从php代码库移植。 我有一个很长的控制器方法，基本上根据我的购物车中的物品计算总价。这是一个直接从php代码移植过来的传统方法。

def total 
    order = @cart.get_or_create_order 
    order_contents = order_contents_for(order) 

    discounts = { 
     events: {}, 
     subjects: {}, 
     products: {} 
    } 

    adjusted_pricing = {} 
    free = false 
    shipping = 0 
    total = 0 

    # Logic for computing shipping price 

    # Construct discount hash 

    order_contents.each do |item| 
     if (item.variant_price.present?) 
     price = item.variant_price 
     end 
     else 
     price = item.price 
     end 

     price_adjustments = {} 
     popped_from = [] 

     # it's the issue due to legacy database structure, 
     # product_id, subject_id and event_id is each column in 
     # the database 
     if (discounts[:products][item.product_id]) 
      price_adjustments = discounts[:products][item.product_id] 
      discounts[:products].delete(item.product_id) 
      popped_from = [:products, item.product_id] 
     elsif (discounts[:subjects][item.subject_id]) 
      price_adjustments = discounts[:subjects][item.subject_id] 
      discounts[:subjects].delete(item.subject_id) 
      popped_from = [:subjects, item.subject_id] 
     elsif (discounts[:events][item.event_id]) 
      price_adjustments = discounts[:events][item.event_id] 
      discounts[:events].delete(item.event_id) 
      popped_from = [:events, item.event_id] 
     end 

     if (adjustment = price_adjustments['$']) 
      adjusted_price = price + adjustment 
     elsif (adjustment = price_adjustments['%']) 
      adjusted_price = price + price * (adjustment/100.0) 
      discounts[popped_from[0]][popped_from[1]] = price_adjustments 
     else 
      adjusted_price = price 
     end 

     adjusted_pricing[item.product_id] = {price: adjusted_price, discount: price - adjusted_price} 

     total += adjusted_price 
    end 

    total += shipping 
end 

上面的代码是一个巨大的一个方法的代码片段，所以我试图重构它，并将其移动到一个模型price_calculator。

def calculate_total_for(order) 
    order_contents = order.current_order_contents 
    product_adjustments = order.product_adjustments 

    shipping = calculate_shipping_price(order_contents, product_adjustments) 

    discounts = construct_discount_hash(product_adjustments) 

    adjusted_pricing = construct_adjusted_price_hash(discounts, order_contents) 

    total_price = adjusted_pricing.inject(0) { |total, (k, v)| total + v[:price] } 

    { 
    total_price: total_price + shipping, 
    shipping: shipping, 
    adjusted_pricing: adjusted_pricing 
    } 
end 

我所做的，基本上，还是在移动之前的巨大的方法变成自己的类的流程，并在类如calculate_shipping_price，construct_discount_hash拆分逻辑到一个单独的私有方法。

我知道这远不是一个好的代码。在可读性方面将其分解为私有方法，但我开始觉得它很难做出测试。希望这里的某个人能给出一个建议或指导，在ruby中重构上面代码的最好方法是什么？ PS：我是ruby/rails的新手，我之前主要使用C＃/ Javascript编写代码，所以有一些习惯用法或ruby方式来处理我不熟悉的东西。

Answer 1

对于您提到的示例，我将使用Extract Class from Method重构并使用Service Object而不是在模型中移动所有内容。

这里是如何做到这一点的概述，当然我离开执行对您：

class Test 
    def total 
    order = @cart.get_or_create_order 
    order_contents = order_contents_for(order) 

    discounts = { 
     events: {}, 
     subjects: {}, 
     products: {} 
    } 

    service = CalculateTotal.new(order, order_contents, discounts) 

    if service.success? 
     # Success logic 
    else 
     flash[:error] = service.error 
     # Failure logic 
    end 
    end 
end 

class CalculateTotal 
    attr_reader :success, :error 

    def initialize(order, order_contents, discounts) 
    @order = order 
    @order_contents = order_contents 
    @discounts = discounts 
    end 

    def call 
    sum_orders + adjusted_prices + shipping 
    end 

    def success? 
    !@error 
    end 

    private 

    def sum_orders 
    # Logic 

    if something_fails 
     @error = 'There was an error calculating the price' 
    end 

    # Logic 
    end 

    def adjusted_prices 
    # Logic 
    end 

    def shipping 
    # Logic 
    end 
end