将大控制器方法重构为模型

Question

我有一个从 php 代码库移植的 rails 应用程序。 我有一个很长的控制器方法，它基本上根据我的购物车中的物品计算总价。这是直接从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