在 rails 中重构 ruby 方法

Question

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

我想稍微重构一下。肯定是 "check if object exists then do something with it" 部分出了问题。我想我可以使用 try 但我不知道它是如何工作的。 sby 能告诉我怎么做吗（使用 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

更新：

一切都在 current_user 上作为实例方法而不是 Notification.class_method 调用。顺便提一句。用户 table 有 "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

，它将引发

但这对你没有多大帮助。

如果你迭代，你将不需要检查是否存在某些东西。您可以迭代，如果关联是 "empty"，则什么也不会发生。

例如，您可以重写 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 可以为 each 调用 checking_and_decreasing_notification 而不仅仅是 first

由于我们在这里看不到完整的代码，所以我必须做出一些假设。该代码看起来不像 OO（您有 self 方法接收所有必需的值作为参数）。我建议您将其重构为：

使该方法成为相关模型的行为（方法）
制作涵盖单一业务运营的服务

顺便说一句：很酷，你使用 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

在 rails 中重构 ruby 方法

refactoring ruby methods in rails

ruby

refactoring

ruby-on-rails