ruby-on-rails - 使用存在条件检查逻辑重构 if-else

Question

我有这两个方法我想重构

def construct_discount_hash(product_adjustments)
  discounts = {
      events: {},
      subjects: {},
      products: {}
  }

  # product_adjustments is a model
  # This is a problem from legacy database structure where event_id, subject_id, product_id is in their own column
  product_adjustments.each do |adjustment|

    if (adjustment.event_id.present?)
      discounts[:events][adjustment.event_id] = {'$' => adjustment.amount, '%' => adjustment.percentage}
    end

    if (adjustment.subject_id.present?)
      discounts[:subjects][adjustment.subject_id] = {'$' => adjustment.amount, '%' => adjustment.percentage}
    end

    if (adjustment.product_id.present?)
        discounts[:products][adjustment.product_id] = {'$' => adjustment.amount, '%' => adjustment.percentage}
    end
  end

  discounts
end

我将在下面的方法中使用上面的方法结果

# discounts is a hash generated from above code, item is a rails model
def calculate_order_content_price(discounts, item)
    product = item.product

    if (item.product_variant.present?)
      price = item.product_variant.price
    else
      price = product.price
    end

    price_adjustments = {}
    popped_from = []

    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][product.subject_id])
      price_adjustments = discounts[:subjects][product.subject_id]
      discounts[:subjects].delete(product.subject_id)
      popped_from = [:subjects, product.subject_id]
    elsif (discounts[:events][product.event_id])
      price_adjustments = discounts[:events][product.event_id]
      discounts[:events].delete(product.event_id)
      popped_from = [:events, product.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

    { price: adjusted_price, discount: (price - adjusted_price) }
end

我从上面的代码知道那里有很多代码味道。首先，我认为 if-else 逻辑可以以某种方式重构。有人可以给我一个建议，我可以用它来重构 if-else 条件吗？我很困惑，因为 if 条件正在检查值是否存在。

任何建议都会有所帮助。

Answer 1

我尝试使用两个助手来稍微简化您的construct_discount_hash:

def amount_and_percentage(adjustment, model)
  { adjustment.attributes["#{model}_id"] => { '$': adjustment.amount, '%': adjustment.percentage } }
end

def construct_discount_hash(product_adjustments)
  product_adjustments.each_with_object({}) do |adjustment, hash|
    case 
    when adjustment.event_id.present?
      hash[:event] = amount_and_percentage(adjustment, 'event')
    when adjustment.subject_id.present?
      hash[:subject] = amount_and_percentage(adjustment, 'subject')
    when adjustment.product_id.present?
      hash[:product] = amount_and_percentage(adjustment, 'product')  
    end
  end
end