ruby-on-rails - 如何重构 after_create 回调？

Question

这是我的 after_create 回调：

after_create { |f| if f.target.class.eql?(Question)  
        if f.target.user != User.current_user
          Notify.create_notify( Notify::QUESTION_FOLLOW, f.target.user, User.current_user, f.target) 
        end
      elsif f.target.class.eql?(User)   
        Notify.create_notify( Notify::USER_FOLLOW, f.target, User.current_user,f.target)  if f.target.can_mail_user(:follower) 
      end 
  } 

我尝试将它移动到一个块中，所以现在它如下所示：

after_create do |f| 
  if f.target.class.eql?(Question)  
    if f.target.user != User.current_user
      Notify.create_notify( Notify::QUESTION_FOLLOW, f.target.user, User.current_user, f.target) 
    end
  elsif f.target.class.eql?(User)   
    Notify.create_notify( Notify::USER_FOLLOW, f.target, User.current_user,f.target)  if f.target.can_mail_user(:follower) 
  end 
end 

我还能做些什么来改进该代码？

Answer 1

大部分应该去Notify。我假设这是一Follow堂课。在重构时要问自己一个有用的问题是“这个类需要知道这种行为吗？”，我认为在这种情况下答案是否定的。尝试做这样的事情：

after_create do |f|
  Notify.create_notify(f.target.user, User.current_user, f.target)
end

...其余的分支逻辑进入 Notify，它的工作是确定要发送哪个通知。

Answer 2

改进它的一种方法是使用 is_a? 方法而不是直接比较类名。此外，变量 f 的描述性不够。最后，我将创建一个新变量来存储 f.target，这样您就不会重复自己。

例如：

after_create do |record| 
  target = record.target
  if target.is_a?(Question) 
    if target.user != User.current_user
      Notify.create_notify( Notify::QUESTION_FOLLOW, target.user, User.current_user, target) 
    end
  elsif target.is_a?(User)   
    Notify.create_notify( Notify::USER_FOLLOW, target, User.current_user, target)  if target.can_mail_user(:follower) 
  end 
end