In a typical Rails app, one ActiveRecord model tends to accumulate a lot of associations and related methods. This is usually the User class; e.g., the User has many posts, comments, contacts, projects, etc. It’s also common to have a few instance methods to filter these associations, e.g., User#unpublished_posts, or User#recent_contacts.

Soon this God class becomes overwhelmed by all of these responsibilities. The intent of this refactoring is to move this behavior to the associated class.

Knowing Too Much

The following ActiveRecord model has a few associations (to keep this example simple), and an instance method to filter one of them.

Question Bidirectional Associations

When a new feature requires an ActiveRecord association, avoid the tendency to automatically make it bidirectional. Bidirectional associations often result in too many parent class responsibilities. Instead, implement these responsibilities in the associated class. It's more work, but your classes will stay small, cohesive, and ungodlike.

A quick note on security with this refactoring: Without chaining through user, the query for projects will not automatically be constrained to the user, leaving this work to be done by the developer. The approach taken here is to pass current_user.id to the Project#active_projects_for method. If current_user#id is nil, this could mistakenly return all projects that are not assigned to any users. This is different from the user constrained approach where the most data would could ever mistakenly return is all of a user’s projects. Its nothing that can’t be explicitly handled, but it is something that AR associations give you for free.

July 22, 2013 at 8:35 am

Jared Carroll says:

@Ben,

Good point. That’s definitely a trade-off of this design. Maybe Project.active_projects_for could raise something in that exceptional situation.

July 22, 2013 at 6:51 pm

Brian Gates says:

You don’t need the user_id argument. Just chain a class method on the associated class to the association: