Unprivatise all the things

Well, not all of them, but some of them.

I don't think there's much reason for these methods to be private.

activerecord/lib/active_record/associations/builder/association.rb

@@ -52,42 +52,40 @@ def valid_options
       Association.valid_options
     end
 
-    private
+    def validate_options
+      options.assert_valid_keys(valid_options)
+    end
 
-      def validate_options
-        options.assert_valid_keys(valid_options)
-      end
+    def define_accessors
+      define_readers
+      define_writers
+    end
 
-      def define_accessors
-        define_readers
-        define_writers
+    def define_readers
+      name = self.name
+      mixin.redefine_method(name) do |*params|
+        association(name).reader(*params)
       end
+    end
 
-      def define_readers
-        name = self.name
-        mixin.redefine_method(name) do |*params|
-          association(name).reader(*params)
-        end
+    def define_writers
+      name = self.name
+      mixin.redefine_method("#{name}=") do |value|
+        association(name).writer(value)
       end
+    end
 
-      def define_writers
-        name = self.name
-        mixin.redefine_method("#{name}=") do |value|
-          association(name).writer(value)
-        end
+    def validate_dependent_option(valid_options)
+      unless valid_options.include? options[:dependent]
+        raise ArgumentError, "The :dependent option must be one of #{valid_options}, but is :#{options[:dependent]}"
       end
 
-      def validate_dependent_option(valid_options)
-        unless valid_options.include? options[:dependent]
-          raise ArgumentError, "The :dependent option must be one of #{valid_options}, but is :#{options[:dependent]}"
-        end
-
-        if options[:dependent] == :restrict
-          ActiveSupport::Deprecation.warn(
-            "The :restrict option is deprecated. Please use :restrict_with_exception instead, which " \
-            "provides the same functionality."
-          )
-        end
+      if options[:dependent] == :restrict
+        ActiveSupport::Deprecation.warn(
+          "The :restrict option is deprecated. Please use :restrict_with_exception instead, which " \
+          "provides the same functionality."
+        )
       end
+    end
   end
 end

activerecord/lib/active_record/associations/builder/belongs_to.rb

@@ -21,71 +21,69 @@ def build
       reflection
     end
 
-    private
-
-      def add_counter_cache_callbacks(reflection)
-        cache_column = reflection.counter_cache_column
-        name         = self.name
+    def add_counter_cache_callbacks(reflection)
+      cache_column = reflection.counter_cache_column
+      name         = self.name
+
+      method_name = "belongs_to_counter_cache_after_create_for_#{name}"
+      mixin.redefine_method(method_name) do
+        record = send(name)
+        record.class.increment_counter(cache_column, record.id) unless record.nil?
+      end
+      model.after_create(method_name)
 
-        method_name = "belongs_to_counter_cache_after_create_for_#{name}"
-        mixin.redefine_method(method_name) do
+      method_name = "belongs_to_counter_cache_before_destroy_for_#{name}"
+      mixin.redefine_method(method_name) do
+        unless marked_for_destruction?
           record = send(name)
-          record.class.increment_counter(cache_column, record.id) unless record.nil?
-        end
-        model.after_create(method_name)
-
-        method_name = "belongs_to_counter_cache_before_destroy_for_#{name}"
-        mixin.redefine_method(method_name) do
-          unless marked_for_destruction?
-            record = send(name)
-            record.class.decrement_counter(cache_column, record.id) unless record.nil?
-          end
+          record.class.decrement_counter(cache_column, record.id) unless record.nil?
         end
-        model.before_destroy(method_name)
-
-        model.send(:module_eval,
-          "#{reflection.class_name}.send(:attr_readonly,\"#{cache_column}\".intern) if defined?(#{reflection.class_name}) && #{reflection.class_name}.respond_to?(:attr_readonly)", __FILE__, __LINE__
-        )
       end
+      model.before_destroy(method_name)
+
+      model.send(:module_eval,
+        "#{reflection.class_name}.send(:attr_readonly,\"#{cache_column}\".intern) if defined?(#{reflection.class_name}) && #{reflection.class_name}.respond_to?(:attr_readonly)", __FILE__, __LINE__
+      )
+    end
 
-      def add_touch_callbacks(reflection)
-        name        = self.name
-        method_name = "belongs_to_touch_after_save_or_destroy_for_#{name}"
-        touch       = options[:touch]
+    def add_touch_callbacks(reflection)
+      name        = self.name
+      method_name = "belongs_to_touch_after_save_or_destroy_for_#{name}"
+      touch       = options[:touch]
 
-        mixin.redefine_method(method_name) do
-          record = send(name)
+      mixin.redefine_method(method_name) do
+        record = send(name)
 
-          unless record.nil?
-            if touch == true
-              record.touch
-            else
-              record.touch(touch)
-            end
+        unless record.nil?
+          if touch == true
+            record.touch
+          else
+            record.touch(touch)
           end
         end
-
-        model.after_save(method_name)
-        model.after_touch(method_name)
-        model.after_destroy(method_name)
       end
 
-      def configure_dependency
-        if dependent = options[:dependent]
-          validate_dependent_option [:destroy, :delete]
+      model.after_save(method_name)
+      model.after_touch(method_name)
+      model.after_destroy(method_name)
+    end
 
-          model.send(:class_eval, <<-eoruby, __FILE__, __LINE__ + 1)
-            def #{dependency_method_name}
-              association(:#{name}).handle_dependency
-            end
-          eoruby
+    def configure_dependency
+      if dependent = options[:dependent]
+        validate_dependent_option [:destroy, :delete]
 
-          model.after_destroy dependency_method_name
-        end
-      end
+        model.send(:class_eval, <<-eoruby, __FILE__, __LINE__ + 1)
+          def #{dependency_method_name}
+            association(:#{name}).handle_dependency
+          end
+        eoruby
 
-      def dependency_method_name
-        "belongs_to_dependent_for_#{name}"
+        model.after_destroy dependency_method_name
       end
+    end
+
+    def dependency_method_name
+      "belongs_to_dependent_for_#{name}"
+    end
   end
 end

activerecord/lib/active_record/associations/builder/collection_association.rb

@@ -34,53 +34,51 @@ def show_deprecation_warnings
       end
     end
 
-    private
-
-      def wrap_block_extension
-        if block_extension
-          @extension_module = mod = Module.new(&block_extension)
-          silence_warnings do
-            model.parent.const_set(extension_module_name, mod)
-          end
-
-          prev_scope = @scope
-
-          if prev_scope
-            @scope = proc { |owner| instance_exec(owner, &prev_scope).extending(mod) }
-          else
-            @scope = proc { extending(mod) }
-          end
+    def wrap_block_extension
+      if block_extension
+        @extension_module = mod = Module.new(&block_extension)
+        silence_warnings do
+          model.parent.const_set(extension_module_name, mod)
         end
-      end
 
-      def extension_module_name
-        @extension_module_name ||= "#{model.name.demodulize}#{name.to_s.camelize}AssociationExtension"
+        prev_scope = @scope
+
+        if prev_scope
+          @scope = proc { |owner| instance_exec(owner, &prev_scope).extending(mod) }
+        else
+          @scope = proc { extending(mod) }
+        end
       end
+    end
 
-      def define_callback(callback_name)
-        full_callback_name = "#{callback_name}_for_#{name}"
+    def extension_module_name
+      @extension_module_name ||= "#{model.name.demodulize}#{name.to_s.camelize}AssociationExtension"
+    end
 
-        # TODO : why do i need method_defined? I think its because of the inheritance chain
-        model.class_attribute full_callback_name.to_sym unless model.method_defined?(full_callback_name)
-        model.send("#{full_callback_name}=", Array(options[callback_name.to_sym]))
-      end
+    def define_callback(callback_name)
+      full_callback_name = "#{callback_name}_for_#{name}"
 
-      def define_readers
-        super
+      # TODO : why do i need method_defined? I think its because of the inheritance chain
+      model.class_attribute full_callback_name.to_sym unless model.method_defined?(full_callback_name)
+      model.send("#{full_callback_name}=", Array(options[callback_name.to_sym]))
+    end
 
-        name = self.name
-        mixin.redefine_method("#{name.to_s.singularize}_ids") do
-          association(name).ids_reader
-        end
+    def define_readers
+      super
+
+      name = self.name
+      mixin.redefine_method("#{name.to_s.singularize}_ids") do
+        association(name).ids_reader
       end
+    end
 
-      def define_writers
-        super
+    def define_writers
+      super
 
-        name = self.name
-        mixin.redefine_method("#{name.to_s.singularize}_ids=") do |ids|
-          association(name).ids_writer(ids)
-        end
+      name = self.name
+      mixin.redefine_method("#{name.to_s.singularize}_ids=") do |ids|
+        association(name).ids_writer(ids)
       end
+    end
   end
 end

activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb

@@ -24,18 +24,16 @@ def show_deprecation_warnings
       end
     end
 
-    private
-
-      def define_destroy_hook
-        name = self.name
-        model.send(:include, Module.new {
-          class_eval <<-RUBY, __FILE__, __LINE__ + 1
-            def destroy_associations
-              association(#{name.to_sym.inspect}).delete_all
-              super
-            end
-          RUBY
-        })
-      end
+    def define_destroy_hook
+      name = self.name
+      model.send(:include, Module.new {
+        class_eval <<-RUBY, __FILE__, __LINE__ + 1
+          def destroy_associations
+            association(#{name.to_sym.inspect}).delete_all
+            super
+          end
+        RUBY
+      })
+    end
   end
 end

activerecord/lib/active_record/associations/builder/has_many.rb

@@ -15,23 +15,21 @@ def build
       reflection
     end
 
-    private
+    def configure_dependency
+      if dependent = options[:dependent]
+        validate_dependent_option [:destroy, :delete_all, :nullify, :restrict, :restrict_with_error, :restrict_with_exception]
 
-      def configure_dependency
-        if dependent = options[:dependent]
-          validate_dependent_option [:destroy, :delete_all, :nullify, :restrict, :restrict_with_error, :restrict_with_exception]
-
-          name = self.name
-          mixin.redefine_method(dependency_method_name) do
-            association(name).handle_dependency
-          end
-
-          model.before_destroy dependency_method_name
+        name = self.name
+        mixin.redefine_method(dependency_method_name) do
+          association(name).handle_dependency
         end
-      end
 
-      def dependency_method_name
-        "has_many_dependent_for_#{name}"
+        model.before_destroy dependency_method_name
       end
+    end
+
+    def dependency_method_name
+      "has_many_dependent_for_#{name}"
+    end
   end
 end

activerecord/lib/active_record/associations/builder/has_one.rb

@@ -21,23 +21,21 @@ def build
       reflection
     end
 
-    private
+    def configure_dependency
+      if dependent = options[:dependent]
+        validate_dependent_option [:destroy, :delete, :nullify, :restrict, :restrict_with_error, :restrict_with_exception]
 
-      def configure_dependency
-        if dependent = options[:dependent]
-          validate_dependent_option [:destroy, :delete, :nullify, :restrict, :restrict_with_error, :restrict_with_exception]
-
-          name = self.name
-          mixin.redefine_method(dependency_method_name) do
-            association(name).handle_dependency
-          end
-
-          model.before_destroy dependency_method_name
+        name = self.name
+        mixin.redefine_method(dependency_method_name) do
+          association(name).handle_dependency
         end
-      end
 
-      def dependency_method_name
-        "has_one_dependent_for_#{name}"
+        model.before_destroy dependency_method_name
       end
+    end
+
+    def dependency_method_name
+      "has_one_dependent_for_#{name}"
+    end
   end
 end

activerecord/lib/active_record/associations/builder/singular_association.rb

@@ -13,22 +13,20 @@ def define_accessors
       define_constructors if constructable?
     end
 
-    private
+    def define_constructors
+      name = self.name
 
-      def define_constructors
-        name = self.name
-
-        mixin.redefine_method("build_#{name}") do |*params, &block|
-          association(name).build(*params, &block)
-        end
+      mixin.redefine_method("build_#{name}") do |*params, &block|
+        association(name).build(*params, &block)
+      end
 
-        mixin.redefine_method("create_#{name}") do |*params, &block|
-          association(name).create(*params, &block)
-        end
+      mixin.redefine_method("create_#{name}") do |*params, &block|
+        association(name).create(*params, &block)
+      end
 
-        mixin.redefine_method("create_#{name}!") do |*params, &block|
-          association(name).create!(*params, &block)
-        end
+      mixin.redefine_method("create_#{name}!") do |*params, &block|
+        association(name).create!(*params, &block)
       end
+    end
   end
 end