While you still hear “fat models, skinny controllers” from time to time, people have generally caught on that not all code needs to be in a model or controller. Fat models don’t happen all at once, though. I find that you add a method here and a method there until you realize that one of your models is well over 200 lines and has too many methods and too little cohesion. Here’s how I approach this problem.
1. Look at the private methods
Assuming you factor shared code between public methods into private methods on your model, they are the best place to find related methods. Do you have two or more public methods using the same private method? Those methods could likely become the public interface for their own focused class. Also, if you have one public method using multiple private methods, it’s possible that method is so complicated it could be a whole class on its own.
Let’s say we have two public methods that share a private method like this:
The subscription_active?
and subscription_expiration_date
methods are prime candidates to get refactored into their own class.
2. Create a new class and call it from the old one
In this example I would make a new class like so:
Then I would make the User
model look like this:
At this point, make sure your tests all still pass. Then, move the tests for these methods from the User
model to tests for the UserSubscriptions
class. Make sure they still pass.
At this point, depending on the size of your code base and the number of callers of these two methods, you may end up stopping here. You may think you haven’t really improved things since you’ve just moved some code around and didn’t reduce the User
model’s public footprint. You did, however, make an obvious place for future developers to add subscription-related code that isn’t inside the User
model. This is an improvement because it discourages people from making the problems in the User
model worse.
3. Change callers to use the new class
If at all possible, though, your goal should be to delete those delegations from the User
model altogether. You do that by changing every instance of user.subscription_active?
to UserSubscriptions.new(user).subscription_active?
. You do the same with subscription_expiration_date
and then you are well on your way to skinny controllers, skinny models.
I hope this helps you reduce the size of classes that get out of control!