Ticket 9900 - declarative callbacks for ActiveRecord::Observers

Ticket 9900 adds a declarative way to define observer callbacks which
seems more in line with the Rails way of thinking.

   http://dev.rubyonrails.org/ticket/9900

The patch is rather simple and the tests are great. Ping for core to
give it a look,

Zach

Personally I prefer the current ability to specify a well-named
callback method. I think this patch would encourage large anonymous
blocks of code that would hinder refactoring, no?

Jeff

Hmm, perhaps you could be more clear? Refactoring in terms of being
able to read the code easily? Or do you mean that when using this
syntax that it would be harder to make changes to it? Additonally, I
think an observer should 'declare' what it is observing, rather than
defining callback methods that happen to be called back by an active
record observer. Also, the blocks of code aren't as anonymous as you
think. They are used to define callback methods that are recognized in
ActiveRecord::Callbacks::CALLBACKS, so debugging the stack trace will
not lead you block invocations, but to the declarations made in the
observer.

Here is why I think refactoring would be easier using the syntax that
is provided in the patch

== Current way of observing

I want to feed a monkey after I create or update one if it is hungry
and was a well behaved monkey

class MonkeyObserver < ActiveRecord::Observer
  def after_validation_on_create(monkey)
    if monkey.hungry?
      ZooKeeper.feed(monkey)
    end
  end

  def after_validation_on_update(monkey)
    if monkey.hungry?
      ZooKeeper.feed(monkey)
    end
  end
end

Lets refactor that so that we aren't duplicating code...

class MonkeyObserver < ActiveRecord::Observer
  def after_validation_on_create(monkey)
    feed_monkey(monkey)
  end

  def after_validation_on_update(monkey)
    feed_monkey(monkey)
  end

  private
  def feed_monkey(monkey)
    if monkey.hungry?
      ZooKeeper.feed(monkey)
    end
  end
end

Personally, I don't like that nearly as much as...

== Declarative Observer

class MonkeyObserver < ActiveRecord::Observer
  after :validation, :on => :create do |monkey|
    if monkey.hungry?
      ZooKeeper.feed(monkey)
    end
  end

  after :validation, :on => :update do |monkey|
    if monkey.hungry?
      ZooKeeper.feed(monkey)
    end
  end
end

After refactoring....

class MonkeyObserver < ActiveRecord::Observer
  after :validation, :on => [:create, :update] do |monkey|
    if monkey.hungry?
      ZooKeeper.feed(monkey)
    end
  end
end

I think the intent of the observer is clearer this way, and the
refactoring is much lighter and cleaner IMO.

Umm..how is that so ?

    CALLBACKS = %w(
      after_find after_initialize before_save after_save before_create
after_create before_update after_update before_validation
      after_validation before_validation_on_create
after_validation_on_create before_validation_on_update
      after_validation_on_update before_destroy after_destroy
    )

Places where the format proposed in this patch works, are dispatcher
callbacks and rescue_from, on top of my head.

Having said that, I don't see any harm in #9900 if it's a simple
patch. Always good to have options :slight_smile:

Also, for those who find the syntax to be uncomfortable, the old way
of simply declaring the callbacks still exists.