重构大型控制器方法中的模型



我有一个从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_priceconstruct_discount_hash

我知道这远非一个好的代码。在可读性方面将其分解为私人方法,但是我开始觉得很难为其进行测试。希望这里的人可以提供建议或指南,在Ruby中重构代码的最佳方法是什么。

ps:我是Ruby/Rails的新手,我以前主要是在C#/JavaScript中代码,因此有一些习语或Ruby-way做我不熟悉的事情。

在您提到的示例中,我将使用方法重构和使用服务对象而不是移动模型中的所有内容。

这是有关如何做的概述,当然,我为您留下了实施:

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

最新更新