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

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