Split static and dynamic attributes into separate classes

lib/factory_girl.rb

@@ -5,6 +5,8 @@ require 'factory_girl/proxy/create'
 require 'factory_girl/proxy/attributes_for'
 require 'factory_girl/factory'
 require 'factory_girl/attribute'
+require 'factory_girl/attribute/static'
+require 'factory_girl/attribute/dynamic'
 require 'factory_girl/sequence'
 require 'factory_girl/aliases'
 

lib/factory_girl/attribute.rb

@@ -7,32 +7,18 @@ class Factory
 
     attr_reader :name
 
-    def initialize (name, static_value, lazy_block)
+    def initialize(name)
-      name = name.to_sym
+      @name = name.to_sym
 
-      if name.to_s =~ /=$/
+      if @name.to_s =~ /=$/
         raise AttributeDefinitionError, 
-          "factory_girl uses 'f.#{name.to_s.chop} value' syntax " +
+          "factory_girl uses 'f.#{@name.to_s.chop} value' syntax " +
-          "rather than 'f.#{name} = value'" 
+          "rather than 'f.#{@name} = value'" 
-      end
-
-      unless static_value.nil? || lazy_block.nil?
-        raise AttributeDefinitionError, "Both value and block given"
-      end
-
-      @name         = name
-      @static_value = static_value
-      @lazy_block   = lazy_block
-    end
-
-    def value(proxy)
-      if @lazy_block.nil?
-        @static_value
-      else
-        @lazy_block.call(proxy)
       end
     end
 
+    def add_to(proxy)
+    end
   end
 
 end

lib/factory_girl/attribute/dynamic.rb

@@ -0,0 +1,17 @@
+class Factory
+  class Attribute #:nodoc:
+
+    class Dynamic < Attribute  #:nodoc:
+
+      def initialize(name, block)
+        super(name)
+        @block = block
+      end
+
+      def add_to(proxy)
+        proxy.set(name, @block.call(proxy))
+      end
+    end
+
+  end
+end

lib/factory_girl/attribute/static.rb

@@ -0,0 +1,17 @@
+class Factory
+  class Attribute #:nodoc:
+
+    class Static < Attribute  #:nodoc:
+
+      def initialize(name, value)
+        super(name)
+        @value = value
+      end
+
+      def add_to(proxy)
+        proxy.set(name, @value)
+      end
+    end
+
+  end
+end

lib/factory_girl/factory.rb

@@ -65,7 +65,15 @@ class Factory
   #   value: (Object)
   #     If no block is given, this value will be used for this attribute.
   def add_attribute (name, value = nil, &block)
-    attribute = Attribute.new(name, value, block)
+    if block_given?
+      if value
+        raise AttributeDefinitionError, "Both value and block given"
+      else
+        attribute = Attribute::Dynamic.new(name, block)
+      end
+    else
+      attribute = Attribute::Static.new(name, value)
+    end
 
     if attribute_defined?(attribute.name)
       raise AttributeDefinitionError, "Attribute already defined: #{name}"
@@ -187,7 +195,7 @@ class Factory
     passed_keys = overrides.keys.collect {|k| Factory.aliases_for(k) }.flatten
     @attributes.each do |attribute|
       unless passed_keys.include?(attribute.name)
-        proxy.set(attribute.name, attribute.value(proxy))
+        attribute.add_to(proxy)
       end
     end
     proxy.result

test/attribute_test.rb

@@ -2,63 +2,31 @@ require(File.join(File.dirname(__FILE__), 'test_helper'))
 
 class AttributeTest < Test::Unit::TestCase
 
-  def setup
-    @proxy = mock('proxy')
-  end
-
   context "an attribute" do
     setup do
       @name  = :user
-      @attr  = Factory::Attribute.new(@name, 'test', nil)
+      @attr  = Factory::Attribute.new(@name)
     end
 
     should "have a name" do
       assert_equal @name, @attr.name
     end
-  end
 
-  context "an attribute with a static value" do
+    should "do nothing when being added to a proxy" do
-    setup do
+      @proxy = mock('proxy')
-      @value = 'test'
+      @proxy.expects(:set).never
-      @attr  = Factory::Attribute.new(:user, @value, nil)
+      @attr.add_to(@proxy)
-    end
-
-    should "return the static value when asked for its value" do
-      assert_equal @value, @attr.value(@proxy)
-    end
-  end
-
-  context "an attribute with a lazy value" do
-    setup do
-      @block = lambda { 'value' }
-      @attr  = Factory::Attribute.new(:user, nil, @block)
-    end
-
-    should "call the block to return a value" do
-      assert_equal 'value', @attr.value(@proxy)
-    end
-
-    should "yield the passed proxy to the block" do
-      @block = lambda {|a| a }
-      @attr  = Factory::Attribute.new(:user, nil, @block)
-      assert_equal @proxy, @attr.value(@proxy)
     end
   end
 
   should "raise an error when defining an attribute writer" do
     assert_raise Factory::AttributeDefinitionError do
-      Factory::Attribute.new('test=', nil, nil)
+      Factory::Attribute.new('test=')
-    end
-  end
-
-  should "not accept both a value parameter and a block" do
-    assert_raise(Factory::AttributeDefinitionError) do
-      Factory::Attribute.new(:name, 'value', lambda {})
     end
   end
 
   should "convert names to symbols" do
-    assert_equal :name, Factory::Attribute.new('name', nil, nil).name
+    assert_equal :name, Factory::Attribute.new('name').name
   end
 
 end

test/dynamic_attribute_test.rb

@@ -0,0 +1,41 @@
+require(File.join(File.dirname(__FILE__), 'test_helper'))
+
+class DynamicAttributeTest < Test::Unit::TestCase
+
+  context "a static attribute" do
+    setup do
+      @name  = :first_name
+      @block = lambda { 'value' }
+      @attr  = Factory::Attribute::Dynamic.new(@name, @block)
+    end
+
+    should "have a name" do
+      assert_equal @name, @attr.name
+    end
+
+    should "call the block to set a value" do
+      @proxy = mock('proxy')
+      @proxy.expects(:set).with(@name, 'value')
+      @attr.add_to(@proxy)
+    end
+
+    should "yield the proxy to the block when adding its value to a proxy" do
+      @block = lambda {|a| a }
+      @attr  = Factory::Attribute::Dynamic.new(:user, @block)
+      @proxy = mock('proxy')
+      @proxy.expects(:set).with(:user, @proxy)
+      @attr.add_to(@proxy)
+    end
+  end
+
+  should "raise an error when defining an attribute writer" do
+    assert_raise Factory::AttributeDefinitionError do
+      Factory::Attribute::Dynamic.new('test=', nil)
+    end
+  end
+
+  should "convert names to symbols" do
+    assert_equal :name, Factory::Attribute::Dynamic.new('name', nil).name
+  end
+
+end

test/factory_test.rb

@@ -67,18 +67,33 @@ class FactoryTest < Test::Unit::TestCase
 
     should "not allow the same attribute to be added twice" do
       assert_raise(Factory::AttributeDefinitionError) do
-        2.times { @factory.add_attribute @name }
+        2.times { @factory.add_attribute :first_name }
       end
     end
 
-    should "create a new attribute when an attribute is defined" do
+    should "add a static attribute when an attribute is defined with a value" do
-      block = lambda {}
       attribute = mock('attribute', :name => :name)
-      Factory::Attribute.
+      Factory::Attribute::Static.
         expects(:new).
-        with(:name, 'value', block).
+        with(:name, 'value').
         returns(attribute)
-      @factory.add_attribute(:name, 'value', &block)
+      @factory.add_attribute(:name, 'value')
+    end
+
+    should "add a dynamic attribute when an attribute is defined with a block" do
+      attribute = mock('attribute', :name => :name)
+      block     = lambda {}
+      Factory::Attribute::Dynamic.
+        expects(:new).
+        with(:name, block).
+        returns(attribute)
+      @factory.add_attribute(:name, &block)
+    end
+
+    should "raise for an attribute with a value and a block" do
+      assert_raise(Factory::AttributeDefinitionError) do
+        @factory.add_attribute(:name, 'value') {}
+      end
     end
 
     context "after adding an attribute" do
@@ -86,12 +101,12 @@ class FactoryTest < Test::Unit::TestCase
         @attribute = mock('attribute')
         @proxy     = mock('proxy')
 
-        @attribute.           stubs(:name).  returns(:name)
+        @attribute.                stubs(:name).  returns(:name)
-        @attribute.           stubs(:value). returns('value')
+        @attribute.                stubs(:add_to)
-        @proxy.               stubs(:set)
+        @proxy.                    stubs(:set)
-        @proxy.               stubs(:result).returns('result')
+        @proxy.                    stubs(:result).returns('result')
-        Factory::Attribute.   stubs(:new).   returns(@attribute)
+        Factory::Attribute::Static.stubs(:new).   returns(@attribute)
-        Factory::Proxy::Build.stubs(:new).   returns(@proxy)
+        Factory::Proxy::Build.     stubs(:new).   returns(@proxy)
 
         @factory.add_attribute(:name, 'value')
       end
@@ -104,13 +119,8 @@ class FactoryTest < Test::Unit::TestCase
         @factory.run(Factory::Proxy::Build, {})
       end
 
-      should "get the value from the attribute when running" do
+      should "add the attribute to the proxy when running" do
-        @attribute.expects(:value).with(@proxy).returns('value')
+        @attribute.expects(:add_to).with(@proxy)
-        @factory.run(Factory::Proxy::Build, {})
-      end
-
-      should "set the value on the proxy when running" do
-        @proxy.expects(:set).with(:name, 'value')
         @factory.run(Factory::Proxy::Build, {})
       end
 
@@ -164,8 +174,11 @@ class FactoryTest < Test::Unit::TestCase
     should "add an attribute using the method name when passed an undefined method" do
       attr  = mock('attribute', :name => :name)
       block = lambda {}
-      Factory::Attribute.expects(:new).with(:name, 'value', block).returns(attr)
+      Factory::Attribute::Static.
-      @factory.send(:name, 'value', &block)
+        expects(:new).
+        with(:name, 'value').
+        returns(attr)
+      @factory.send(:name, 'value')
       assert @factory.attributes.include?(attr)
     end
 
@@ -296,13 +309,10 @@ class FactoryTest < Test::Unit::TestCase
 
   context "after defining a factory" do
     setup do
-      @name = :user
+      @name    = :user
-      Factory.define(@name) do |f|
+      @factory = mock('factory')
-        f.first_name 'First'
+
-        f.last_name  'Last'
+      Factory.factories[@name] = @factory
-        f.email      'name@example.com'
-      end
-      @factory = Factory.factories[@name]
     end
 
     teardown { Factory.factories.clear }
@@ -345,6 +355,7 @@ class FactoryTest < Test::Unit::TestCase
       end
 
       should "recognize either 'name' or :name for Factory.#{method}" do
+        @factory.stubs(:run)
         assert_nothing_raised { Factory.send(method, @name.to_s) }
         assert_nothing_raised { Factory.send(method, @name.to_sym) }
       end

test/static_attribute_test.rb

@@ -0,0 +1,33 @@
+require(File.join(File.dirname(__FILE__), 'test_helper'))
+
+class StaticAttributeTest < Test::Unit::TestCase
+
+  context "a static attribute" do
+    setup do
+      @name  = :first_name
+      @value = 'John'
+      @attr  = Factory::Attribute::Static.new(@name, @value)
+    end
+
+    should "have a name" do
+      assert_equal @name, @attr.name
+    end
+
+    should "set its static value on a proxy" do
+      @proxy = mock('proxy')
+      @proxy.expects(:set).with(@name, @value)
+      @attr.add_to(@proxy)
+    end
+  end
+
+  should "raise an error when defining an attribute writer" do
+    assert_raise Factory::AttributeDefinitionError do
+      Factory::Attribute::Static.new('test=', nil)
+    end
+  end
+
+  should "convert names to symbols" do
+    assert_equal :name, Factory::Attribute::Static.new('name', nil).name
+  end
+
+end