重構軌方法

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