Rails: monkey-patching ActiveRecord::Base vs creating a Module

Question

I am reading through The Rails 4 way (by Obie Fernandez), a well-known book about Rails, and from what I've read so far, I can highly recommend it.

However, there is an example section 9.2.7.1: Multiple Callback Methods in One Class that confuses me:

Bear with me, to make the problem clear for everyone, I have replicated the steps the book describes in this question.

---

The section talks about Active Record callbacks (before_create, before_update and so on), and that it is possible to create a class that handles multiple callbacks for you. The listed code is as follows:

class Auditor
   def initialize(audit_log)
      @audit_log = audit_log
   end

   def after_create(model)
      @audit_log.created(model.inspect)
   end

   def after_update(model)
      @audit_log.updated(model.inspect)
   end

   def after_destroy(model)
      @audit_log.destroyed(model.inspect)
   end
end

The book says then that to add this audit logging to an Active Record class, you would do the following:

class Account < ActiveRecord::Base
   after_create Auditor.new(DEFAULT_AUDIT_LOG)
   after_update Auditor.new(DEFAULT_AUDIT_LOG)
   after_destroy Auditor.new(DEFAULT_AUDIT_LOG)
   ...
end

The book then notes that this code is very ugly, having to add three Auditors on three lines, and that it not DRY. It then goes ahead and tells us that to solve this problem, we should monkey-patch an acts_as_audited method into the Active Record::Base object, as follows:

(the book suggests putting this file in /lib/core_ext/active_record_base.rb)

class ActiveRecord::Base
   def self.acts_as_audited(audit_log=DEFAULT_AUDIT_LOG)
      auditor = Auditor.new(audit_log)
      after_create auditor
      after_update auditor
      after_destroy auditor
   end
end

which enables you to write the Account Model class as follows:

class Account < ActiveRecord::Base
   acts_as_audited
   ...
end

---

Before reading the book, I have already made something similar that adds functionality to multiple Active Record models. The technique I used was to create a Module. To stay with the example, what I have done was similar to:

(I would put this file inside /app/models/auditable.rb)

module Auditable
   def self.included(base)
      @audit_log = base.audit_log || DEFAULT_AUDIT_LOG #The base class can override it if wanted, by specifying a self.audit_log before including this module
      base.after_create audit_after_create
      base.after_update audit_after_update
      base.after_destroy audit_after_destroy
   end

   def audit_after_create
      @audit_log.created(self.inspect)
   end

   def audit_after_update
      @audit_log.updated(self.inspect)
   end

   def audit_after_destroy
      @audit_log.destroyed(self.inspect)
   end
end

Note that this file both replaces the Auditor and the monkey-patched ActiveRecord::Base method. The Account class would then look like:

class Account < ActiveRecord::Base
   include Auditable
   ...
end

---

Now you've read both the way the book does it, and the way I would have done it in the past. My question: Which version is more sustainable in the long-term? I realize that this is a slightly opinionated question, just like everything about Rails, but to keep it answerable, I basically want to know:

• Why would you want to monkey-patch ActiveRecord::Base directly, over creating and including a Module?

Answer 1

I would go for the module for a few reasons.

Its obvious; that is to say, I can quickly find the code that defines this behavior. In acts_as_* I don't know if its from some gem, library code, or defined within this class. There could be implications about it being overridden or piggy-backed in the call-stack.

Its portable. It uses method calls that are commonly defined in libraries that define callbacks. You could conceivably distribute and use this library in non-active-record objects.

It avoids the addition of unnecessary code on the static level. I'm a fan of having less code to manage (less code to break). I like using Ruby's niceties without doing to much to force it to be "nicer" than it already it is.

In a monkey-patch setting you are tying the code to a class or module that could go away and there are scenarios where it would fail silently until your class can't call acts_as_*.

One downfall of the portability argument is the testing argument. In which case I would say you can write your code to protect against portability, or fail early with smart warnings about what will and won't work when used portably.