ruby-on-rails - 重构 rails 方法

Question

我想知道，我该如何重构这段代码，因为它看起来不太好阅读和理解

def next_payment_price
    price = self.plan_price
    price = discounted_price if self.coupon && self.coupon_duration.nil? && self.coupon_discount != 100
    price = discounted_price if self.coupon && self.coupon_duration.present? && self.coupon_discount != 100 && ((self.created_at + 14.days + self.coupon_duration.month)  > Time.now )
    price
end

def discounted_price
    self.plan_price - ((self.plan_price * self.coupon_discount) / 100)
end

Answer 1

我认为您可以使用较小的方法来更好地阅读

  def next_payment_price
    correct_discount? && correct_coupon? ? discounted_price : self.plan_price
  end

  def expired_coupon?
    (self.created_at + 14.days + self.coupon_duration.month)  < Time.now
  end

  def correct_coupon?
    self.coupon_duration.nil? || (self.coupon_duration && !expired_coupon?)
  end

  def correct_discount?
    self.coupon && self.coupon_discount && self.coupon_discount < 100
  end

  def discounted_price
    self.plan_price - self.plan_price * self.coupon_discount / 100
  end

Answer 2

我建议您在coupon模型中创建一些小方法，从而赋予对象更多含义，例如：

def is_less_than_100?
  self.coupon_discount != 100
end

def is_date_bigger_than_today?
  (self.created_at + 14.days + self.coupon_duration.month)  > Time.now
end

这样你的代码会更少，而且更容易理解：

price = discounted_price if self.is_less_than_100? and self.is_date_bigger_than_today?

PS：名字只是为了说明目的。我想你明白了

Answer 3

如果您也将过期逻辑提取到方法中怎么办？

    def not_expired?
        return false if self.coupon_duration.nil?
        ((self.created_at + 14.days + self.coupon_duration.month)  > Time.now )
    end

然后：

    def next_payment_price
        price = self.plan_price
        price = discounted_price if self.coupon? and not_expired? ...
    end