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:
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
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!