Yes, there are a lot of opportunities for refactoring in the associations code. But, I have two problems with the all-or-nothing approach. First, refactoring is *by definition* not all or nothing; it is small, consistent improvements. Turning away an improvement because it doesn't fix absolutely everything is a step back into waterfall thinking. Second, I'm happy to continue refactoring associations, and anything else I find that can use it. Well designed code pleases me; poorly designed code irks me. But, I'm not inclined to spend my free time working on refactorings only to see them rejected because they're "just refactorings."
On Apr 23, 4:48 am, Chris Cruft <c...@hapgoods.com> wrote: > Having just spent a lot of time wading through the HMT and HOT code, > I'll agree that there is a lot of room for improvement w.r.t. > clarity. And if there is a general agreement that a refactoring is > needed then the longer we wait the tougher the job will be. I would > encourage Adam's effort. > > However, having already had discussions with several of you in the > past, I think we all know the problem doesn't stop with HMT and HOT - > it's the entire inheritance hierarchy and mixin structure of > associations. So Adam, are you prepared to expand the scope of your > refactoring to tackle the bigger job? Because I can see some downside > to a partial refactoring that leaves a code base with no track record > and few acolytes. Then finishing the job could actually be harder. > > But overall, I think the clock is ticking on this one. Associations > code is already big and complex. Every accepted patch just adds to > the inertia of the current structure that I think everyone knows needs > to be re-thought. > > --Chris > > On Apr 22, 4:28 pm, Chad Woolley <thewoolley...@gmail.com> wrote: > > > On Wed, Apr 22, 2009 at 10:51 AM, Pratik <pratikn...@gmail.com> wrote: > > > > Which problem is it solving for you ? Do you see any performance gains > > > that I'm missing ? Do you believe if this is making any code easier to > > > understand ? Could you please provide failing tests which are to be > > > fixed by the refactoring in question ? > > > I believe that Adam already addresses many of these points in his original > > post: > > > "This inheritance relationship has already caused its share of bugs, as > > mentioned by commenters on the ticket. I fixed two > > here:http://rails.lighthouseapp.com/projects/8994/tickets/895-has_one-thro.... > > More importantly, the added complexity created by importing all of the > > collection logic and interface into a non-collection association class > > just adds to rigidity and potential for odd bugs in the future." > > > It is hard to write a failing test for bugs that don't exist, but the > > existence of prior already-fixed bugs should be considered as a > > motivation for doing the refactor. > > > Furthermore, the existence of bugs isn't normally the primary > > motivation for refactoring - but they are a symptom of a needed > > refactoring. Complex, hard-to-understand code is a direct motivation > > for refactoring, which I think is the point here. > > > -- Chad --~--~---------~--~----~------------~-------~--~----~ 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+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---