Refactor dependency check validation

Move the logic for validation check to the same method, and cache
dependent option in a variable to reuse inside the dependency
configuration methods instead of relying on the options hash.

4 files changed, 19 insertions, 28 deletions

diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb
index 4086485ee2..c3f32b5ed9 100644
--- a/activerecord/lib/active_record/associations/builder/association.rb
+++ b/activerecord/lib/active_record/associations/builder/association.rb
@@ -77,17 +77,14 @@ module ActiveRecord::Associations::Builder
         end
       end
 
-      def print_valid_dependent_options(options)
-        options.map {|s| s.to_s.insert 0, ":" }.
-          to_sentence(:words_connector => ", ",
-                      :two_words_connector => ' or ',
-                      :last_word_connector => ' or ')
-      end
+      def check_valid_dependent!(dependent, valid_options)
+        unless valid_options.include?(dependent)
+          valid_options_message = valid_options.map(&:inspect).to_sentence(
+            words_connector: ', ', two_words_connector: ' or ', last_word_connector: ' or ')
 
-      def check_dependent_valid(valid_options)
-        return if valid_options.include?(options[:dependent])
-        raise ArgumentError, "The :dependent option expects either " \
-          "#{print_valid_dependent_options(valid_options)} (#{options[:dependent].inspect})"
+          raise ArgumentError, "The :dependent option expects either " \
+            "#{valid_options_message} (#{dependent.inspect})"
+        end
       end
 
       def dependent_restrict_raises?
diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb
index 277bec6f72..405dfc97b7 100644
--- a/activerecord/lib/active_record/associations/builder/belongs_to.rb
+++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb
@@ -72,19 +72,17 @@ module ActiveRecord::Associations::Builder
       end
 
       def configure_dependency
-        if options[:dependent]
+        if dependent = options[:dependent]
+          check_valid_dependent! dependent, [:destroy, :delete]
 
-          check_dependent_valid [:destroy, :delete]
-
-          method_name = "belongs_to_dependent_#{options[:dependent]}_for_#{name}"
+          method_name = "belongs_to_dependent_#{dependent}_for_#{name}"
           model.send(:class_eval, <<-eoruby, __FILE__, __LINE__ + 1)
             def #{method_name}
               association = #{name}
-              association.#{options[:dependent]} if association
+              association.#{dependent} if association
             end
           eoruby
           model.after_destroy method_name
-
         end
       end
   end
diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb
index 6920bf0633..d0bdfa7d9c 100644
--- a/activerecord/lib/active_record/associations/builder/has_many.rb
+++ b/activerecord/lib/active_record/associations/builder/has_many.rb
@@ -19,14 +19,12 @@ module ActiveRecord::Associations::Builder
     private
 
       def configure_dependency
-        if options[:dependent]
+        if dependent = options[:dependent]
+          check_valid_dependent! dependent, [:destroy, :delete_all, :nullify, :restrict]
+          dependent_restrict_deprecation_warning if dependent == :restrict
 
-          check_dependent_valid [:destroy, :delete_all, :nullify, :restrict]
-
-          dependent_restrict_deprecation_warning if options[:dependent] == :restrict
-          send("define_#{options[:dependent]}_dependency_method")
+          send("define_#{dependent}_dependency_method")
           model.before_destroy dependency_method_name
-
         end
       end
 
diff --git a/activerecord/lib/active_record/associations/builder/has_one.rb b/activerecord/lib/active_record/associations/builder/has_one.rb
index 442fd50b1d..cd6d9c871b 100644
--- a/activerecord/lib/active_record/associations/builder/has_one.rb
+++ b/activerecord/lib/active_record/associations/builder/has_one.rb
@@ -25,14 +25,12 @@ module ActiveRecord::Associations::Builder
     private
 
       def configure_dependency
-        if options[:dependent]
+        if dependent = options[:dependent]
+          check_valid_dependent! dependent, [:destroy, :delete, :nullify, :restrict]
+          dependent_restrict_deprecation_warning if dependent == :restrict
 
-          check_dependent_valid [:destroy, :delete, :nullify, :restrict]
-
-          dependent_restrict_deprecation_warning if options[:dependent] == :restrict
-          send("define_#{options[:dependent]}_dependency_method")
+          send("define_#{dependent}_dependency_method")
           model.before_destroy dependency_method_name
-
         end
       end