Move callback validation into Callback

lib/factory_girl/attribute_list.rb

@@ -19,10 +19,6 @@ def define_attribute(attribute)
     end
 
     def add_callback(name, &block)
-      unless valid_callback_names.include?(name.to_sym)
-        raise InvalidCallbackNameError, "#{name} is not a valid callback name. Valid callback names are #{valid_callback_names.inspect}"
-      end
-
       @callbacks << Callback.new(name.to_sym, block)
     end
 
@@ -66,10 +62,6 @@ def size
 
     private
 
-    def valid_callback_names
-      [:after_build, :after_create, :after_stub]
-    end
-
     def add_attribute(attribute)
       delete_attribute(attribute.name) if overridable?
 

lib/factory_girl/callback.rb

@@ -1,10 +1,22 @@
 module FactoryGirl
   class Callback
+    VALID_NAMES = [:after_build, :after_create, :after_stub].freeze
+
     attr_reader :name, :block
 
     def initialize(name, block)
       @name  = name.to_sym
       @block = block
+      check_name
+    end
+
+    private
+
+    def check_name
+      unless VALID_NAMES.include?(name)
+        raise InvalidCallbackNameError, "#{name} is not a valid callback name. " +
+          "Valid callback names are #{VALID_NAMES.inspect}"
+      end
     end
   end
 end

spec/factory_girl/attribute_list_spec.rb

@@ -63,31 +63,13 @@
 describe FactoryGirl::AttributeList, "#add_callback" do
   let(:proxy_class) { mock("klass") }
   let(:proxy) { FactoryGirl::Proxy.new(proxy_class) }
-  let(:valid_callback_names) { [:after_create, :after_build, :after_stub] }
-  let(:invalid_callback_names) { [:before_create, :before_build, :bogus] }
 
   it "allows for defining adding a callback" do
     subject.add_callback(:after_create) { "Called after_create" }
 
     subject.callbacks.first.name.should == :after_create
     subject.callbacks.first.block.call.should == "Called after_create"
   end
-
-  it "allows valid callback names to be assigned" do
-    valid_callback_names.each do |callback_name|
-      expect do
-        subject.add_callback(callback_name) { "great name!" }
-      end.to_not raise_error(FactoryGirl::InvalidCallbackNameError)
-    end
-  end
-
-  it "raises if an invalid callback name is assigned" do
-    invalid_callback_names.each do |callback_name|
-      expect do
-        subject.add_callback(callback_name) { "great name!" }
-      end.to raise_error(FactoryGirl::InvalidCallbackNameError, "#{callback_name} is not a valid callback name. Valid callback names are [:after_build, :after_create, :after_stub]")
-    end
-  end
 end
 
 describe FactoryGirl::AttributeList, "#apply_attributes" do

spec/factory_girl/callback_spec.rb

@@ -1,16 +1,23 @@
 require 'spec_helper'
 
 describe FactoryGirl::Callback do
-  let(:name)  { :after_create }
-  let(:block) { proc { "block" } }
-  let(:proxy) { stub("proxy") }
+  it "has a name" do
+    FactoryGirl::Callback.new(:after_create, lambda {}).name.should == :after_create
+  end
 
-  subject     { FactoryGirl::Callback.new(name, block) }
+  it "converts strings to symbols" do
+    FactoryGirl::Callback.new("after_create", lambda {}).name.should == :after_create
+  end
 
-  its(:name)  { should == name }
-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 {}) }.
+        to_not raise_error(FactoryGirl::InvalidCallbackNameError)
+    end
+  end
 
-describe FactoryGirl::Callback, "with a string name" do
-  subject    { FactoryGirl::Callback.new("name", nil) }
-  its(:name) { should == :name }
+  it "raises if an invalid callback name is assigned" do
+    expect { FactoryGirl::Callback.new(:magic_fairies, lambda {}) }.
+      to raise_error(FactoryGirl::InvalidCallbackNameError, /magic_fairies is not a valid callback name/)
+  end
 end