重构轨方法

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? 

P.S .:这些名称仅用于说明目的。我认为你有这个想法

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