braintree / curator

Model and repository framework
MIT License
351 stars 39 forks source link

Add ActiveModel::Callbacks to Curator::Model #31

Open jamesottaway opened 11 years ago

jamesottaway commented 11 years ago

I'm just getting started with Curator, which I'm really excited about, but one interesting feature which I think would make it even more compelling is to add callbacks or hooks, similar to what ActiveRecord has done.

http://api.rubyonrails.org/classes/ActiveRecord/Callbacks.html

In my case, I've got a Post model with a title and slug, and I'm trying to default the slug if one was not explicitly passed, but respect an explicit one if it is passed in. My code looks something like this:

class Post
  include Curator::Model

  attr_accessor :title, :slug

  def initialize(args={})
    args[:slug] ||= args[:title].to_slug
    super(args)
  end
end

I'd love to be able to hook into an after_create callback which would mean I don't need to override initialize while still calling super after I've made cahnges to args.

Does this seem like something valuable for Curator?

pgr0ss commented 11 years ago

Hi James,

I'm not sure callbacks belong in curator. The goal is to keep domain objects separated from persistence, and methods like after_create tie them together. That said, the repositories are classes that you write, so you are free to add your own hooks there. For example, you could override save.

jamesottaway commented 11 years ago

Yeah, you're right, I'm probably still trying to break out of my ActiveRecord habit. Obviously tying the Model and Repository together defeats the purpose of Curator, but now that I think about it more, my idea was more to do with wrapping a callback around just the initialize method rather than going overboard like ActiveRecord callbacks do.

I doubled checked the docs I was looking at yesterday and realised that the ActiveModel::Callbacks module is more in line with what I'm thinking, due to the fact that Curator::Model handles the initialize method and I still feel dirty about overriding it with a call to super.

I'll send through a pull request shortly once I've validated I'm still not crazy. Thanks @pgr0ss.