用存在条件检查的逻辑重构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

从上面的代码中我知道有很多代码的味道。首先，我认为如果-否则的逻辑可以被重构。有人能给我一个建议吗?我可以用它来重构如果-否则的条件？我很困惑，因为如果条件是检查值的存在性。

任何建议都会有帮助。

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

Answer 2

对你来说是事实。

def construct_discount_hash(product_adjustments)
  # 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|
    adjustement_hash = {'$' => adjustment.amount, '%' => adjustment.percentage} if adjustement
    (discounts ||= {})[:events][adjustment.event_id] = adjustement_hash if adjustment.event_id.present?
    (discounts ||= {})[:subjects][adjustment.subject_id] = adjustement_hash if adjustment.subject_id.present?
    (discounts ||= {})[:products][adjustment.product_id] = adjustement_hash if adjustment.product_id.present?
  end
  discounts
end

我希望它能帮到你。玩得开心。