Use objects, not structs

lib/factory_girl/attribute_list.rb

@@ -18,8 +18,8 @@ def define_attribute(attribute)
       add_attribute attribute
     end
 
-    def add_callback(name, &block)
-      @callbacks << Callback.new(name.to_sym, block)
+    def add_callback(callback)
+      @callbacks << callback
     end
 
     def each(&block)
@@ -31,7 +31,7 @@ def attribute_defined?(attribute_name)
     end
 
     def apply_attributes(attributes_to_apply)
-      attributes_to_apply.callbacks.each { |callback| add_callback(callback.name, &callback.block) }
+      attributes_to_apply.callbacks.each { |callback| add_callback(callback) }
       new_attributes = []
 
       attributes_to_apply.each do |attribute|

lib/factory_girl/callback.rb

@@ -10,6 +10,14 @@ def initialize(name, block)
       check_name
     end
 
+    def run(instance, proxy)
+      case block.arity
+      when 1 then block.call(instance)
+      when 2 then block.call(instance, proxy)
+      else block.call
+      end
+    end
+
     private
 
     def check_name

lib/factory_girl/factory.rb

@@ -90,7 +90,7 @@ def define_trait(trait)
     end
 
     def add_callback(name, &block)
-      @attribute_list.add_callback(name, &block)
+      @attribute_list.add_callback(Callback.new(name, block))
     end
 
     def attributes
@@ -102,7 +102,7 @@ def attributes
 
     def run(proxy_class, overrides) #:nodoc:
       proxy = proxy_class.new(build_class)
-      callbacks.each { |callback| proxy.add_callback(callback.name, callback.block) }
+      callbacks.each { |callback| proxy.add_callback(callback) }
       overrides = symbolize_keys(overrides)
 
       attributes.each do |attribute|

lib/factory_girl/proxy.rb

@@ -4,6 +4,7 @@ class Proxy #:nodoc:
     attr_reader :callbacks
 
     def initialize(klass)
+      @callbacks = {}
     end
 
     def get(attribute)
@@ -15,20 +16,15 @@ def set(attribute, value, ignored = false)
     def associate(name, factory, attributes)
     end
 
-    def add_callback(name, block)
-      @callbacks ||= {}
-      @callbacks[name] ||= []
-      @callbacks[name] << block
+    def add_callback(callback)
+      @callbacks[callback.name] ||= []
+      @callbacks[callback.name] << callback
     end
 
     def run_callbacks(name)
-      if @callbacks && @callbacks[name]
-        @callbacks[name].each do |block|
-          case block.arity
-          when 0 then block.call
-          when 2 then block.call(@instance, self)
-          else block.call(@instance)
-          end
+      if @callbacks[name]
+        @callbacks[name].each do |callback|
+          callback.run(@instance, self)
         end
       end
     end

lib/factory_girl/proxy/attributes_for.rb

@@ -2,6 +2,7 @@ module FactoryGirl
   class Proxy #:nodoc:
     class AttributesFor < Proxy #:nodoc:
       def initialize(klass)
+        super(klass)
         @hash = {}
         @ignored_attributes = {}
       end

lib/factory_girl/proxy/build.rb

@@ -2,6 +2,7 @@ module FactoryGirl
   class Proxy #:nodoc:
     class Build < Proxy #:nodoc:
       def initialize(klass)
+        super(klass)
         @instance = klass.new
         @ignored_attributes = {}
       end

lib/factory_girl/proxy/stub.rb

@@ -4,6 +4,7 @@ class Stub < Proxy #:nodoc:
       @@next_id = 1000
 
       def initialize(klass)
+        super(klass)
         @instance = klass.new
         @ignored_attributes = {}
         @instance.id = next_id

lib/factory_girl/trait.rb

@@ -15,7 +15,7 @@ def define_attribute(attribute)
     end
 
     def add_callback(name, &block)
-      @attribute_list.add_callback(name, &block)
+      @attribute_list.add_callback(Callback.new(name, block))
     end
 
     def attributes

spec/factory_girl/attribute_list_spec.rb

@@ -65,10 +65,10 @@
   let(:proxy) { FactoryGirl::Proxy.new(proxy_class) }
 
   it "allows for defining adding a callback" do
-    subject.add_callback(:after_create) { "Called after_create" }
+    subject.add_callback(FactoryGirl::Callback.new(:after_create, lambda { "Called after_create" }))
 
     subject.callbacks.first.name.should == :after_create
-    subject.callbacks.first.block.call.should == "Called after_create"
+    subject.callbacks.first.run(nil, nil).should == "Called after_create"
   end
 end
 

spec/factory_girl/callback_spec.rb

@@ -9,6 +9,24 @@
     FactoryGirl::Callback.new("after_create", lambda {}).name.should == :after_create
   end
 
+  it "runs its block with no parameters" do
+    ran_with = nil
+    FactoryGirl::Callback.new(:after_create, lambda { ran_with = [] }).run(:one, :two)
+    ran_with.should == []
+  end
+
+  it "runs its block with one parameter" do
+    ran_with = nil
+    FactoryGirl::Callback.new(:after_create, lambda { |one| ran_with = [one] }).run(:one, :two)
+    ran_with.should == [:one]
+  end
+
+  it "runs its block with two parameters" do
+    ran_with = nil
+    FactoryGirl::Callback.new(:after_create, lambda { |one, two| ran_with = [one, two] }).run(:one, :two)
+    ran_with.should == [:one, :two]
+  end
+
   it "allows valid callback names to be assigned" do
     FactoryGirl::Callback::VALID_NAMES.each do |callback_name|
       expect { FactoryGirl::Callback.new(callback_name, lambda {}) }.

spec/factory_girl/proxy/create_spec.rb

@@ -25,18 +25,16 @@
   end
 
   context "callback execution order" do
-    let(:after_build_callback)  { stub("after_build callback", :foo => nil) }
-    let(:after_create_callback) { stub("after_create callback", :foo => nil) }
-    let(:callback_sequence)     { sequence("after_* callbacks") }
-
     it "runs after_build callbacks before after_create callbacks" do
-      subject.add_callback(:after_build,  proc { after_build_callback.foo })
-      subject.add_callback(:after_create, proc { after_create_callback.foo })
-
-      after_build_callback.expects(:foo).once.in_sequence(callback_sequence)
-      after_create_callback.expects(:foo).once.in_sequence(callback_sequence)
+      ran = []
+      after_create = FactoryGirl::Callback.new(:after_create, lambda { ran << :after_create })
+      after_build = FactoryGirl::Callback.new(:after_build, lambda { ran << :after_build })
+      subject.add_callback(after_create)
+      subject.add_callback(after_build)
 
       subject.result(nil)
+
+      ran.should == [:after_build, :after_create]
     end
   end
 end

spec/factory_girl/proxy_spec.rb

@@ -23,62 +23,49 @@
   end
 
   describe "when adding callbacks" do
-    let(:block_1) { proc { "block 1" } }
-    let(:block_2) { proc { "block 2" } }
-
     it "adds a callback" do
-      subject.add_callback(:after_create, block_1)
-      subject.callbacks[:after_create].should be_eql([block_1])
+      callback = FactoryGirl::Callback.new(:after_create, lambda {})
+      subject.add_callback(callback)
+      subject.callbacks[:after_create].should == [callback]
     end
 
     it "adds multiple callbacks of the same name" do
-      subject.add_callback(:after_create, block_1)
-      subject.add_callback(:after_create, block_2)
-      subject.callbacks[:after_create].should be_eql([block_1, block_2])
+      one = FactoryGirl::Callback.new(:after_create, lambda {})
+      two = FactoryGirl::Callback.new(:after_create, lambda {})
+      subject.add_callback(one)
+      subject.add_callback(two)
+      subject.callbacks[:after_create].should == [one, two]
     end
 
     it "adds multiple callbacks with different names" do
-      subject.add_callback(:after_create, block_1)
-      subject.add_callback(:after_build,  block_2)
-      subject.callbacks[:after_create].should be_eql([block_1])
-      subject.callbacks[:after_build].should be_eql([block_2])
+      after_create = FactoryGirl::Callback.new(:after_create, lambda {})
+      after_build = FactoryGirl::Callback.new(:after_build, lambda {})
+      subject.add_callback(after_create)
+      subject.add_callback(after_build)
+      subject.callbacks[:after_create].should == [after_create]
+      subject.callbacks[:after_build].should == [after_build]
     end
   end
 
   describe "when running callbacks" do
-    let(:object_1_within_callback) { stub("call_in_create", :foo => true) }
-    let(:object_2_within_callback) { stub("call_in_create", :foo => true) }
-
     it "runs all callbacks with a given name" do
-      subject.add_callback(:after_create, proc { object_1_within_callback.foo })
-      subject.add_callback(:after_create, proc { object_2_within_callback.foo })
+      ran = []
+      one = FactoryGirl::Callback.new(:after_create, lambda { ran << :one })
+      two = FactoryGirl::Callback.new(:after_create, lambda { ran << :two })
+      subject.add_callback(one)
+      subject.add_callback(two)
       subject.run_callbacks(:after_create)
-      object_1_within_callback.should have_received(:foo).once
-      object_2_within_callback.should have_received(:foo).once
+      ran.should == [:one, :two]
     end
 
     it "only runs callbacks with a given name" do
-      subject.add_callback(:after_create, proc { object_1_within_callback.foo })
-      subject.add_callback(:after_build,  proc { object_2_within_callback.foo })
-      subject.run_callbacks(:after_create)
-      object_1_within_callback.should have_received(:foo).once
-      object_2_within_callback.should have_received(:foo).never
-    end
-
-    it "passes in the instance if the block takes an argument" do
-      subject.instance_variable_set("@instance", object_1_within_callback)
-      subject.add_callback(:after_create, proc {|spy| spy.foo })
-      subject.run_callbacks(:after_create)
-      object_1_within_callback.should have_received(:foo).once
-    end
-
-    it "passes in the instance and the proxy if the block takes two arguments" do
-      subject.instance_variable_set("@instance", object_1_within_callback)
-      proxy_instance = nil
-      subject.add_callback(:after_create, proc {|spy, proxy| spy.foo; proxy_instance = proxy })
+      ran = []
+      after_create = FactoryGirl::Callback.new(:after_create, lambda { ran << :after_create })
+      after_build = FactoryGirl::Callback.new(:after_build, lambda { ran << :after_build })
+      subject.add_callback(after_create)
+      subject.add_callback(after_build)
       subject.run_callbacks(:after_create)
-      object_1_within_callback.should have_received(:foo).once
-      proxy_instance.should == subject
+      ran.should == [:after_create]
     end
   end
 end

spec/support/shared_examples/proxy.rb

@@ -114,7 +114,7 @@
   let(:callback) { stub("#{callback_name} callback", :foo => nil) }
 
   before do
-    subject.add_callback(callback_name, proc { callback.foo })
+    subject.add_callback(FactoryGirl::Callback.new(callback_name, proc { callback.foo }))
   end
 
   it "runs the #{callback_name} callback" do