Rails core - Dec 2008 - HasOneThroughAssociation should not derive from HasManyThroughAssociation

I see what these two classes have in common, but I don''t like that
they share implementation via an inheritance relationship,
particularly given how badly this violates Liskov substitutability.
So, I moved the shared functionality into a module and included that
module into both HasManyThroughAssociation and
HasOneThroughAssociation, as well as making HasOneThroughAssociation a
subclass of HasOneAssociation.  This makes the inheritance hierarchy
more intuitive, and cleans up the code a bit, since there''s now no
need for overloads of collection-specific methods in a non-collection
association class.  I also eliminated an extraneous database call
caused (I believe) by the association setter forcing a reload of
HasOneThroughAssociation objects.

Ticket for this patch is at
http://rails.lighthouseapp.com/projects/8994/tickets/1642-hasonethroughassociation-should-not-be-a-child-of-hasmanythroughassociation
.

My initial motivation for the above patch was a patch to allow
association proxies to correctly respond to methods defined by the
#method_missing method on their targets.  My initial attempts were all
complicated by the fact that HasOneThroughAssociation inherited some
collection-specific jimcrackery from HasManyThroughAssociation.  With
the above patch in place, the solution became much simpler.

Ticket for this second patch is at
http://rails.lighthouseapp.com/projects/8994/tickets/1643-association-proxies-should-correctly-respond-to-method-defined-via-method_missing
.  Two things to note about this patch: 1) it will cause a test
failure if applied without the previous patch, due to the issue
mentioned above; 2) I haven''t looked in detail at potential
performance implications of using #include? rather than #respond_to?

I''ve also included a third patch to fix a test failure that happens
only when has_many_through_associations_test.rb is run by itself, and
some other minor cleanup refactorings.  Ticket for this final patch is
at
http://rails.lighthouseapp.com/projects/8994/tickets/1644-minor-refactorings-and-order-dependent-test-failure-fix
.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

+1 for cleaning up this mess.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

+1

I pushed for this same refactoring a couple of months ago to fix some
issues with assignment to the proxies.  It''s overdue.

-Chris

On Dec 27, 9:44 am, Adam <amilli...@pivotallabs.com>
wrote:
> I see what these two classes have in common, but I don''t like that
> they share implementation via an inheritance relationship,
> particularly given how badly this violates Liskov substitutability.
> So, I moved the shared functionality into a module and included that
> module into both HasManyThroughAssociation and
> HasOneThroughAssociation, as well as making HasOneThroughAssociation a
> subclass of HasOneAssociation.  This makes the inheritance hierarchy
> more intuitive, and cleans up the code a bit, since there''s now no
> need for overloads of collection-specific methods in a non-collection
> association class.  I also eliminated an extraneous database call
> caused (I believe) by the association setter forcing a reload of
> HasOneThroughAssociation objects.
>
> Ticket for this patch is
athttp://rails.lighthouseapp.com/projects/8994/tickets/1642-hasonethrou...
> .
>
> My initial motivation for the above patch was a patch to allow
> association proxies to correctly respond to methods defined by the
> #method_missing method on their targets.  My initial attempts were all
> complicated by the fact that HasOneThroughAssociation inherited some
> collection-specific jimcrackery from HasManyThroughAssociation.  With
> the above patch in place, the solution became much simpler.
>
> Ticket for this second patch is
athttp://rails.lighthouseapp.com/projects/8994/tickets/1643-association...
> .  Two things to note about this patch: 1) it will cause a test
> failure if applied without the previous patch, due to the issue
> mentioned above; 2) I haven''t looked in detail at potential
> performance implications of using #include? rather than #respond_to?
>
> I''ve also included a third patch to fix a test failure that
happens
> only when has_many_through_associations_test.rb is run by itself, and
> some other minor cleanup refactorings.  Ticket for this final patch is
> athttp://rails.lighthouseapp.com/projects/8994/tickets/1644-minor-refac...
> .
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---