If you’re wondering why I’ve got fat-model obsession, there’s one simple reason - CodeClimate is a highly addictive and enjoyable way to learn just how much your code sucks while telling you where exactly you should be focussing your improvements and once you get started, you just can’t stop. Basically, refactoring extensively so that all your classes are tiny and focussed, and the accompanying feeling of freedom and fluidity that comes from this drives your code quality higher and higher.
People have talked at length about the benefits of SRP but all that reading hadn’t prepared me for the actual experience of whittling classes down to doing just one thing.
- Firstly, you can write really focussed tesets. Now more than half of the files in my models directory have nothing really to do with Rails. As POROs, testing them becomes super fast.
- The elimination of churn from the classes is the biggest source of relief for me. Once the class is specced and written, it almost never changes and this gives me a lot more confidence that changes I make in one part of the code won’t have unintended consequences elsewhere.
The whole experience has been so fundamentally mind altering that I would recommend everyone try out CodeClimate.
During this process, naturally, I learned a lot about keeping models focussed on one thing. CodeClimate’s biggest problem always has been with the models that have an embedded workflow in them, and I figured it might be worthwhile to try and extract the workflow into an object all its own. Here’s my story.
To start with, I have the following scenario. The model in question is the Quotation model, which handles the creation of quotations to be sent to clients. The quotations go through various stages, first being approved or rejected by an admin, then being sent to the client and finally being paid. The original model took care of defining and persisting the attributes. In addition it could answer various questions about what all could be done with the quotation in whatever state it happened to be in, as well as handling all state transitions (including sending email and so on).
Broadly, the class looked like this
The Quotation class is the God class of this application. It has a trip associated with it, a duration, is pooled with other Quotations in shared cabs, it has several fields which represent various orthagonal states (lead quality is one state, the workflow which leads to payment and closure is another), it shows up in the users account and so on. The whole class is very complex and was begging to be ripped apart, scoring as it did, a big fat F on CodeClimate.
So, among other things (such as creating separate scope and search classes), I started to extract a workflow class from this class using the dependency injection technique I talked about in my previous post. As the model currently is, there is a very tight coupling between the Quotation class and its workflow. It is not also possible to have more than one workflow field in the model. In addition, there is no way to have separate workflows based on say, the user associated with the quotation (i.e. different criteria and workflows for different classes of users like guest user, trial user, premium user, etc.) I am happy to report that ripping apart the class and using dependency injection has solved all these problems.
So, the first thing I did was to extract a QuotationStatusPolicy class which would handle all the questions about whether a given quotation was approvable, sendable and so on. i.e.
Using this method, it becomes trivially simple to use a different status policy class for various classes of users.
Now, we need to rip out the heart of the beast, the actual workflow itself. This is done as follows
The last thing thats left is to wire up these new classes into the Quotation class. i.e.
All tests passing? We’re good to go!