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.