Refactoring ActiveRecord's private methods

As it currently stands, ActiveRecord has alot of private and protected methods in the Base class.

ActiveRecord::Base.methods.size

=> 427

ActiveRecord::Base.protected_methods.size

=> 32

ActiveRecord::Base.private_methods.size

=> 193

I really loved the suggestion by Courtenay in Refactoring AR::Base.find (http://groups.google.com/group/rubyonrails-core/ browse_thread/thread/2ff2b3ce6732096f/776dae05ffa2d445). It not only makes extending find calls easier, but all wraps up all the find logic in one class.

I whipped up a quick example patch for refactoring the calculations library today. Its still rough and alot of the large methods in there can now easily be decomposed into short understandable private methods within the class. I'm planning on refactoring callbacks and validations similarly to the way I refactored calculations.

Calculations Patch: Parked at Loopia

Is this a good or bad idea? Is there a good reason for the flat namespace ActiveRecord has?

Calculations Patch: http://pastie.caboo.se/97034

Is this a good or bad idea?

I find this a good idea - the Calculations module is a mixin for models, and the Calculation class does the dirty work.

Is there a good reason for the flat namespace ActiveRecord has?

It probably started out flat to avoid the dreaded premature optimization, then gradually spun out of control.

What I'm mostly excited about is having the freedom to add a ton more private methods. Private methods are not used to frequently in base because they do blot the amount of private methods. This leads to all the code being stuffed in the one method. If you use a class to encapsulate logic, code can be split up into lots of private methods that makes things a bit more understandable. (This hasn't been include in the patch I posted)

Is this a good or bad idea? Is there a good reason for the flat namespace ActiveRecord has?

We're always kinda loathe to apply patches which 'just' refactor the implementation of a given area without some kind of functional change in the area. We've been burned in the past by needlessly breaking plugins between point releases and generally this is where its come from.

2.0 is pretty close now, but I definitely have a few refactorings in mind once that's out of the way, especially around query generation. Such as losing all this stuff.

          add_joins!(sql, options, scope)           add_conditions!(sql, options[:conditions], scope)

          add_group!(sql, options[:group], scope)           add_order!(sql, options[:order], scope)           add_limit!(sql, options, scope)           add_lock!(sql, options, scope)

But between now and then I'd prefer to pass on changes like this.

I'll look into it as well once 2.0 is out. Thanks