Moved attribute validation into the Attribute class

lib/factory_girl/attribute.rb

@@ -1,12 +1,28 @@
 class Factory
 
+  class AttributeDefinitionError < RuntimeError
+  end
+  
   class Attribute #:nodoc:
 
     attr_reader :name
-    attr_writer :static_value, :lazy_block
 
-    def initialize (name)
+    def initialize (name, static_value, lazy_block)
-      @name = name
+      name = name.to_sym
+
+      if name.to_s =~ /=$/
+        raise AttributeDefinitionError, 
+          "factory_girl uses 'f.#{name.to_s.chop} value' syntax " +
+          "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)

lib/factory_girl/factory.rb

@@ -1,8 +1,5 @@
 class Factory
   
-  class AttributeDefinitionError < RuntimeError
-  end
-  
   cattr_accessor :factories, :sequences #:nodoc:
   self.factories = {}
   self.sequences = {}
@@ -93,25 +90,8 @@ class Factory
   #   value: (Object)
   #     If no block is given, this value will be used for this attribute.
   def add_attribute (name, value = nil, &block)
-    name = name.to_sym
+    attribute = Attribute.new(name, value, block)
-
-    if name.to_s =~ /=$/
-      raise AttributeDefinitionError, 
-        "factory_girl uses 'f.#{name.to_s.chop} #{value}' syntax " +
-        "rather than 'f.#{name} #{value}'" 
-    end
-
-    attribute = Attribute.new(name)
     @attributes << attribute
-    
-    if block_given?
-      unless value.nil?
-        raise ArgumentError, "Both value and block given"
-      end
-      attribute.lazy_block = block
-    else
-      attribute.static_value = value
-    end
   end
 
   # Calls add_attribute using the missing method name as the name of the

test/attribute_test.rb

@@ -2,48 +2,69 @@ require(File.join(File.dirname(__FILE__), 'test_helper'))
 
 class AttributeTest < Test::Unit::TestCase
 
+  def setup
+    @proxy = mock('attribute-proxy')
+  end
+
   context "an attribute" do
 
     setup do
       @name  = :user
-      @proxy = mock('attribute-proxy')
+      @attr  = Factory::Attribute.new(@name, 'test', nil)
-      @attr  = Factory::Attribute.new(@name)
     end
 
     should "have a name" do
       assert_equal @name, @attr.name
     end
 
-    context "after setting a static value" do
+  end
 
-      setup do
+  context "an attribute with a static value" do
-        @value             = 'test'
-        @attr.static_value = @value
-      end
-
-      should "return the value" do
-        assert_equal @value, @attr.value(@proxy)
-      end
 
+    setup do
+      @value = 'test'
+      @attr  = Factory::Attribute.new(:user, @value, nil)
     end
 
-    context "after setting a lazy value" do
+    should "return the value" do
-
+      assert_equal @value, @attr.value(@proxy)
-      setup do
-        @attr.lazy_block = lambda { 'value' }
-      end
-
-      should "call the block to return a value" do
-        assert_equal 'value', @attr.value(@proxy)
-      end
-
-      should "yield the attribute proxy to the block" do
-        @attr.lazy_block = lambda {|a| a }
-        assert_equal @proxy, @attr.value(@proxy)
-      end
-
     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 attribute 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)
+    end
+  end
+
+  should "not allow attributes to be added with 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
+  end
+
 end

test/factory_test.rb

@@ -53,14 +53,6 @@ class FactoryTest < Test::Unit::TestCase
 
   end
   
-  should "raise an error when defining a factory when using attribute setters" do
-    assert_raise Factory::AttributeDefinitionError do
-      Factory.define(:user) do |f|
-        f.name = 'test'
-      end
-    end
-  end
-
   context "defining a sequence" do
 
     setup do
@@ -223,12 +215,6 @@ class FactoryTest < Test::Unit::TestCase
       assert_equal @value, @factory.attributes_for[@attr]
     end
 
-    should "not allow attributes to be added with both a value parameter and a block" do
-      assert_raise(ArgumentError) do
-        @factory.add_attribute(:name, 'value') {}
-      end
-    end
-
     should "allow attributes to be added with strings as names" do
       @factory.add_attribute('name', 'value')
       assert_equal 'value', @factory.attributes_for[:name]