在rails中重構ruby方法

Question

我的rails應用程序中的通知模型中有以下代碼。

我想重構一下。可以肯定的是，這個「檢查對象是否存在，然後對它做些什麼」部分是錯誤的。我想我可以使用try，但我不知道它是如何工作的。可以告訴我該怎麼做（與try或者如果有更好的方法，那麼）？

關於其他代碼的任何其他意見？

#check and decrease chat notification that happens between 2 given users (since 1v1 chat notification number max limit is 1) 
def self.decreasing_chat_notification_number(current_user, user) 
    if self.between_chat_recipient(current_user, user).unchecked.any? 
    notification = self.between_chat_recipient(current_user, user).unchecked.first 
    checking_and_decreasing_notification(notification) 
    end 
end 

#check and decrease all the task notifications that happens between 2 given users 
def self.decreasing_task_notification_number(current_user, user) 
    if self.task.between_other_recipient(current_user, user).unchecked 
    self.task.between_other_recipient(current_user, user).unchecked.each do |notification| 
     checking_and_decreasing_notification(notification) 
    end 
    end 
end 

#check and decrease all the post notifications that happened on a given post 
def self.decreeasing_post_notification_number(current_user, post) 
    if self.post.this_post_comments(current_user, post).unchecked 
    self.post.this_post_comments(current_user, post).unchecked.each do |notification| 
     checking_and_decreasing_notification(notification) 
    end 
    end 
end 

private 

def check_notification #chat notification gets checked 
    if self.checked_at == nil 
    update_attributes(checked_at: Time.zone.now) 
    end 
end 

def checking_and_decreasing_notification(notification) 
    notification.check_notification 
    current_user.decrease_new_other_notifications 
    current_user.decreased_other_number_pusher 
end 

範圍：

scope :not_chat, -> { where.not(notifiable_type: "Message") } 
scope :chat, -> { where(notifiable_type: "Message") } 
scope :task, -> { where(notifiable_type: "Task") } 
scope :post, -> { where(notifiable_type: "Post") } 
scope :checked, -> { where.not(checked_at: nil) } 
scope :unchecked, -> { where(checked_at: nil) } 
scope :this_post_comments, -> (recipient_id, post_id) do 
    where("notifications.recipient_id = ? AND notifications.notifiable_id = ?", recipient_id, post_id) 
end 
scope :between_chat_recipient, -> (recipient_id, sender_id) do 
    where("notifications.recipient_id = ? AND notifications.sender_id = ? AND notifications.notifiable_type = ?", recipient_id, sender_id, "Message") 
end 
scope :between_other_recipient, -> (recipient_id, sender_id) do 
    where("notifications.recipient_id = ? AND notifications.sender_id = ?", recipient_id, sender_id) 
end 

UPDATE：

一切都呼籲CURRENT_USER爲實例方法，而不是Notification.class_method。順便說一句。用戶表具有"new_chat_notification",default: 0和"new_other_notification", default: 0屬性來計算當前通知的數量。

user.rb

#check and decrease chat notification that happens between 2 given users (max 1) 
    def decreasing_chat_notification_number(user) 
    notification = self.notifications.between_chat_recipient(user).unchecked.first 
    self.checking_and_decreasing_notification(notification) if notification.present? 
    end 

    #check and decrease task notifications that happens between 2 given users 
    def decreasing_task_notification_number(user) 
    self.notifications.task.between_other_recipient(user).unchecked.each do |notification| 
     self.checking_and_decreasing_notification(notification) 
    end 
    end 

    #check and decrease the post notification that belongs to a given post 
    def decreasing_post_notification_number(post_id) 
    self.notifications.this_post_comments(post_id).unchecked.each do |notification| 
     self.checking_and_decreasing_notification(notification) 
    end 
    end 

    def checking_and_decreasing_notification(notification) 
    notification.check_notification 
    if notification.notifiable_type == "Message" 
     self.decrease_new_chat_notifications 
     self.decreased_chat_number_pusher 
    else 
     self.decrease_new_other_notifications 
     self.decreased_other_number_pusher 
    end 
    end 

    def decrease_new_chat_notifications 
    decrement!(:new_chat_notification) if self.new_chat_notification > 0 
    end 


    def decrease_new_other_notifications 
    decrement!(:new_other_notification) if self.new_other_notification > 0 
    end 

    def decreased_chat_number_pusher 
    number = self.new_chat_notification 
    Pusher['private-'+ self.id.to_s].trigger('new_chat_notification', {:number => number}) 
    end 

    def decreased_other_number_pusher 
    number = self.new_other_notification 
    Pusher['private-'+ self.id.to_s].trigger('new_other_notification', {:number => number}) 
    end 

notification.rb裏

scope :not_chat, -> { where.not(notifiable_type: "Message") } 
scope :chat, -> { where(notifiable_type: "Message") } 
scope :task, -> { where(notifiable_type: "Task") } 
scope :post, -> { where(notifiable_type: "Post") } 
scope :checked, -> { where.not(checked_at: nil) } 
scope :unchecked, -> { where(checked_at: nil) } 

scope :this_post_comments, -> (post_id) do 
    where("notifications.notifiable_id = ?", post_id) 
end 

scope :between_chat_recipient, -> (sender_id) do 
    where("notifications.sender_id = ? AND notifications.notifiable_type = ?", sender_id, "Message") 
end 

scope :between_other_recipient, -> (sender_id) do 
    where("notifications.sender_id = ? AND notifications.notifiable_type != ?", sender_id, "Message") 
end 

def check_notification #chat notification gets checked 
    update_attribute(:checked_at, Time.zone.now) if self.checked_at.nil? 
end 

Answer 1

您可以使用try嘗試發送消息的對象。如果對象響應它，那麼它將被執行，否則將被忽略。

string = "foo" 
string.try(:length) # => 3 
nil.try(:length) => nil 

我建議你用try!來代替。它會提高，如果接收器不迴應消息和不是nil

但是這不會幫助你在這裏。

如果你迭代，你不需要檢查是否存在。你可以迭代，如果關聯是「空的」，那麼什麼都不會發生。

因此，例如，你可以重寫decreeasing_post_notification_number：

def self.decreeasing_post_notification_number(current_user, post) 
    self.post.this_post_comments(current_user, post).unchecked.each do |notification| 
    checking_and_decreasing_notification(notification) 
    end 
end 

也是一樣decreasing_chat_notification_number IFF它是確定調用checking_and_decreasing_notification爲each，而不僅僅是first

由於我們沒有看到完整的代碼這裏我必須做一些假設。該代碼不看OO（你有self方法接收所有需要的值作爲參數）。我建議你把它重構爲之一：

• 使相關車型的這個方法的行爲（方法）
• 使覆蓋單一經營

BTW服務：酷了您使用Time.zone.now而不僅僅是Time.now！

Answer 2

我會去與裏面的幫手移動檢查：

def checking_and_decreasing_notification(notification) 
    return if notification.nil? # here we defend from bad input 

    notification.check_notification 
    current_user.decrease_new_other_notifications 
    current_user.decreased_other_number_pusher 
end 

現在是安全的，稱其爲：

def self.decreasing_chat_notification_number(current_user, user) 
    # first below is safe, since `unchecked` returns a relation 
    # first called on relation will return nil on empty set 
    checking_and_decreasing_notification \ 
     self.between_chat_recipient(current_user, user).unchecked.first 
    end 
end 

旁註：中if後綴形式可能是更優雅在這裏：

def check_notification #chat notification gets checked 
    update_attributes(checked_at: Time.zone.now) if self.checked_at.nil? 
end