Ruby 帮忙看下这段代码是否有更好的实现方式

loveltyoic · 2014年07月09日 · 最后由 loveltyoic 回复于 2014年07月11日 · 3396 次阅读

先描述下需求:项目中用 sidekiq 做后台任务,大概有十几个xxxJob 的 class,每个 job 都定义好了 perform 方法。 现在需要对每个 job 做些 log 操作。 我的做法是定义一个 module,然后让每个 job include 这个 module。

module JobLogger
  extend ActiveSupport::Concern

  included do
    alias_method :perform_without_log, :perform
    undef perform
    def perform(params)
      perform_without_log(params)
      log操作 
    end
  end
end

这么做的问题是,在每个 xxxJob 里,必须要像下面这样

class XXXJob
  def perform
    xxx
  end
  include JobLogger
end

感觉把 include 写在中间有些别扭,有没有更优雅的实现方式?

module JobLogger
  def perform
    log操作
  end
end

class XXXJob
  include JobLogger
  def perform
    xxx
    super
  end
end

没验证,大致如上

#1 楼 @serco 每个方法都要修改,觉得还没现在的做法好呢。而且 perform 会抛出异常,这是需要在 log 里处理的,这也没法实现。

好像基本差不多了,继承是肯定不能接受的。 如果真要改进的话,我的看法是,相比把动作隐含在included中,我比较偏向于明确地表达。

module JobLogger
  extend ActiveSupport::Concern

  module ClassMethods
    def hook_log
      alias_method :perform_without_log, :perform
      define_method :perform do |params|
        perform_without_log params
        log_it
      end
    end
  end

end

class XXXJob
  include JobLogger

  def perform
    xxx
  end

  hook_log
end

@leomayleomay 那个是 Rails 的,Sidekiq 不必然加载 Rails。

class XXXJob
  prepend JobLogger
  def perform
    xxx
  end
end
module JobLogger
  def perform
     puts 'do someting before'
     super
     puts 'do someting after'
  end
end

仅限 ruby2.0

#5 楼 @billy 好像也是 ActiveSupport 提供的,这倒不是问题。 不过这貌似也没有根本的改进吧。

有点违反最小化类职责,Concern 应该不知道其他类的方法。 如果非 job 类也要使用你的 JobLogger,会不会出错?测试代码貌似也有点麻烦。 Corcern 主是用来给 Model Controller 减肥用的,这个为什么要用 Concern 来实现?好像普通模块更好。

#6 楼 @saiga 就是这个,原来 2.0 提供了这么方便的方法啊。

require 'active_support/concern'
module JobLogger
  extend ActiveSupport::Concern

  def self.included(base)
    base.extend(ClassMethods)
  end

  module ClassMethods
    def create_perform(&block)
      define_method :perform do |&block|
        puts :log_start
        yield 
        puts :log_end
      end
    end
  end
end

class A
  include JobLogger
  create_perform { puts :hello }
end

#8 楼 @winnie JobLogger 就是给 Job 用的,Concern 是感觉他的语法更简练,应该不算滥用吧

#5 楼 @billy

一样都是把 constants 放到链的上端,我倾向不用看上去 fancy 但是没有可读性的方式。

@saiga 的方式是我喜欢的,易读且容易维护

@leomayleomay 不好意思,我没看到这里已经有 Concern 了,那 alias_method_chain 在这是肯定可以使用的。

@loveltyoic 其实不用 Concern 更好,普通手段完全可以实现,因为任务是在 Rails 之外的。没必要为了这个增加对 ActiveSupport 的依赖。

@_kaichen 别急,写得这么好还急啥。

#14 楼 @_kaichen 长知识啦,感谢!

需要 登录 后方可回复, 如果你还没有账号请 注册新账号