From 377bece684715edd802ad73fee739669d15eafa6 Mon Sep 17 00:00:00 2001 From: eileencodes Date: Tue, 29 Jul 2014 13:56:39 -0400 Subject: Refactor join_keys to remove complex conditionals Pushing conditionals down to through reflection Only the through association needs the part of this conditional that deals with belongs to and polymorphic? so that can be pushed down into the ThroughReflection reducing the conditionals. Remove conditional because we can delegate join keys to source reflection Remove need for source_macro checking By adding join_id_for to the other reflections we remove the need to cehck against the source_macro and can reduce the conditioanl from the original join_id_for(owner) Using polymorphism instead of testing the source_macro This case statement in join_association is almost exactly the same as the original join_keys code. Testing taht theory by creating a new join_dependency_keys(assoc_klass) method. Refactor join_keys further to be more concise Fixed format of "#:nodoc:" to "# :nodoc:" where I added them to this file. --- .../join_dependency/join_association.rb | 11 ++--- activerecord/lib/active_record/reflection.rb | 47 ++++++++++++---------- 2 files changed, 29 insertions(+), 29 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index 719eff9acc..a9d1099871 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -37,14 +37,9 @@ module ActiveRecord table = tables.shift klass = reflection.klass - case reflection.source_macro - when :belongs_to - key = reflection.association_primary_key - foreign_key = reflection.foreign_key - else - key = reflection.foreign_key - foreign_key = reflection.active_record_primary_key - end + join_keys = reflection.join_keys(klass) + key = join_keys.key + foreign_key = join_keys.foreign_key constraint = build_constraint(klass, table, key, foreign_table, foreign_key) diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 21da011e8a..575e588f35 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -149,19 +149,10 @@ module ActiveRecord JoinKeys = Struct.new(:key, :foreign_key) # :nodoc: def join_keys(assoc_klass) - if source_macro == :belongs_to - if polymorphic? - reflection_key = association_primary_key(assoc_klass) - else - reflection_key = association_primary_key - end - reflection_foreign_key = foreign_key - else - reflection_foreign_key = active_record_primary_key - reflection_key = foreign_key - end - JoinKeys.new(reflection_key, reflection_foreign_key) + JoinKeys.new(foreign_key, active_record_primary_key) end + + def source_macro; macro; end end # Base class for AggregateReflection and AssociationReflection. Objects of # AggregateReflection and AssociationReflection are returned by the Reflection::ClassMethods. @@ -354,9 +345,8 @@ Joining, Preloading and eager loading of these associations is deprecated and wi end alias :check_eager_loadable! :check_preloadable! - def join_id_for(owner) #:nodoc: - key = (source_macro == :belongs_to) ? foreign_key : active_record_primary_key - owner[key] + def join_id_for(owner) # :nodoc: + owner[active_record_primary_key] end def through_reflection @@ -383,8 +373,6 @@ Joining, Preloading and eager loading of these associations is deprecated and wi scope ? [[scope]] : [[]] end - def source_macro; macro; end - def has_inverse? inverse_name end @@ -574,7 +562,7 @@ Joining, Preloading and eager loading of these associations is deprecated and wi end end - class HasManyReflection < AssociationReflection #:nodoc: + class HasManyReflection < AssociationReflection # :nodoc: def initialize(name, scope, options, active_record) super(name, scope, options, active_record) end @@ -584,7 +572,7 @@ Joining, Preloading and eager loading of these associations is deprecated and wi def collection?; true; end end - class HasOneReflection < AssociationReflection #:nodoc: + class HasOneReflection < AssociationReflection # :nodoc: def initialize(name, scope, options, active_record) super(name, scope, options, active_record) end @@ -594,7 +582,7 @@ Joining, Preloading and eager loading of these associations is deprecated and wi def has_one?; true; end end - class BelongsToReflection < AssociationReflection #:nodoc: + class BelongsToReflection < AssociationReflection # :nodoc: def initialize(name, scope, options, active_record) super(name, scope, options, active_record) end @@ -602,9 +590,18 @@ Joining, Preloading and eager loading of these associations is deprecated and wi def macro; :belongs_to; end def belongs_to?; true; end + + def join_keys(assoc_klass) + key = polymorphic? ? association_primary_key(assoc_klass) : association_primary_key + JoinKeys.new(key, foreign_key) + end + + def join_id_for(owner) # :nodoc: + owner[foreign_key] + end end - class HasAndBelongsToManyReflection < AssociationReflection #:nodoc: + class HasAndBelongsToManyReflection < AssociationReflection # :nodoc: def initialize(name, scope, options, active_record) super end @@ -735,6 +732,10 @@ Joining, Preloading and eager loading of these associations is deprecated and wi end end + def join_keys(assoc_klass) + source_reflection.join_keys(assoc_klass) + end + # The macro used by the source association def source_macro source_reflection.source_macro @@ -802,6 +803,10 @@ directive on your declaration like: through_reflection.options end + def join_id_for(owner) # :nodoc: + source_reflection.join_id_for(owner) + end + def check_validity! if through_reflection.nil? raise HasManyThroughAssociationNotFoundError.new(active_record.name, self) -- cgit v1.2.3