We'll respond shortly.
When I introduce a programmer to Rails I encourage them to read the article Skinny Controller, Fat Model. My only complaint about this article is that it applies the Skinny/Fat Pattern in a vague way, proceeding as if by intuition. I’ve found instead that there is a formulaic way to produce excellent Controller code–the
The Skinny/Fat pattern is typically stated in terms of Lines of Code. Your Controllers should have few Lines of Code; where there are many, move them to the Model. This rule has more exceptions than it has applications, so we can state the intent of the Pattern more precisely in terms of Abstraction Boundaries. Remove all Business Logic from your Controllers and put it in the model. Your Controllers are only responsible for mapping between URLs (including other HTTP Request data), coordinating with your Models and your Views, and channeling that back to an HTTP response. Along the way, Controllers may do Access Control, but little more. These instructions are precise, but following them requires intuition and subtle reasoning. The purpose of this post is to avoid subtle heuristics like Abstraction and Code Complexity. There is a Formula. Let’s see an example that follows the Formula:
def create model = Model.new(params[:model]) raise SecurityTransgressionError.new unless logged_in_user.can_update?(model) if model.save render ... else ... end end
What’s going on here? The Action channels user input to the model. It raises an Exception if the user lacks permission to perform this Action. Validation is performed by the model. Given the results, the Controller renders the appropriate Template to the HTTP Response.
Note a few things about this example. All of the access control rules (i.e., Business Logic) are specified in the Model (in this case, in the can_update? method on User). All data validation rules (again, Business Logic) are specified in the Model. There are three benefits to this approach: 1) the Logic in the Model is easier to test as it’s isolated. 2) Our Abstraction Boundaries are respected. 3) Our Controller is brief, i.e. Skinny. The resulting Controller some call anorexic. But I think it is like a conductor: all of the rhythm and melody comes from the players but the conductor keeps the whole orchestra in sync. But metaphors, testing strategies, and hoity-toity talk of Abstraction Boundaries aside, this example is good code because it follows the Formula.
Let’s consider a more sophisticated example.
Suppose that when creating a model of type Model, we always also want to create another type, DependentModel. We might be tempted to write something like:
def create model = Model.new(params[:model]) dependent_model = model.dependents.build(params[:dependent_model]) if model.save ... end
Two Models created in one Action! This looks nothing like the Formula. Just push the creation of the DependentModel into Model and we have:
def create model = Model.new(params[:model]) if model.save ... end
Voila! The Formula.
Let’s consider another Action, one of the non-side-effecting persuasion.
def edit model = Model.find(params[:id]) raise SecurityTransgressionError.new unless logged_in_user.can_update?(model) @colors = Color.find :all end
We can see the same access control pattern as in the previous example, but absent is any input validation as the user is not modifying anything. Something new here is the assignment of @colors. Typically, when we are displaying something to the user (in this case, an edit form), we will be listing various data; in this case, these data are the possible colors a model can be. Years ago, DHH issued a bull stating that “Thou Shalt Not Call Find in Thine Template”. This commandment is not without its problems, but that’s an issue for another blog post. This loading of the data is part of the Formula.
Let’s take this edit action one step further. Suppose Users of type Admin can assign a certain set of colors to a Model, but other Users can select from a smaller set. Let’s rewrite edit to support these new business rules:
def edit model = Model.find(params[:id]) raise SecurityTransgressionError.new unless logged_in_user.can_update?(model) @colors = logged_in_user.admin?? Color.fabulous_colors : Color.drab_colors end
Something has gone wrong here: we’ve put Business Logic in the Controller, ack! But rather than appeal to some ethereal Abstraction Boundaries, just notice that it doesn’t follow the formula exemplified above–it’s got an extra conditional in the assignment of @colors. Let’s redesign this a bit, just aiming to adhere to the Formula:
def edit model = Model.find(params[:id]) raise SecurityTransgressionError.new unless logged_in_user.can_update?(model) @colors = Color.find_by_user_role(logged_in_user.role) end
Much better. We’ve put the rules of determining which color set to select based into the Model, and we have no more Control Flow than our original definition of edit above. This is Skinny as can be. We haven’t violated any Abstraction Boundaries. The color logic is easier to Unit Test. But these ends can be accomplished on auto-pilot just by following the Formula.
Let’s complexify this example one step further.
def edit ... if logged_in_user.admin? render :action => 'edit_admin' else render :action => 'edit_normal' end end
This is not Skinny at all. But, rendering different templates based on the User role is not Business Logic per-se. Rather, it is logic concerning what information is directed to the HTTP response. This properly belongs in the Controller. We could push this into the model:
render :action => model.edit_action_for_user_role(logged_in_user.role)
But this would make the model know about the View, which is unethical. We could use something like the Visitor Pattern to maintain abstraction boundaries but that is ridiculous overkill here. We could put our Fat edit action on a diet by breaking the two conditions into different edit actions on different Controllers. This approach is, perhaps, more RESTful in spirit:
class AdminModelsController ... end class UserModelsController ... end
All of these concerns: violating Abstraction Boundaries, Lines of Code, the complexity of the Visitor Pattern, whether one approach is more RESTful than another… in the end it’s just a judgement-call between unappealing alternatives, and no two programmers will make the same choice, and I on any given day will choose variously. So just follow the Formula. Break it out into two controllers so we can have two edit actions, each well behaved.
Let’s look at a ‘grammar’ for the Formula, and we’ll soon be done.
def non_side_effecting_action model = Model.find(params[:id]) raise SecurityTransgressionError.new unless logged_in_user.can_action?(model) @assign1 = Foo.find :all ... @assignN = Bar.find_for_user(logged_in_user) end def side_effecting_action model = Model.new(params[:model]) # or: model = Model.find(params[:id]); model.attributes = params[:model] raise SecurityTransgressionError.new unless logged_in_user.can_action?(model) if model.save @assign1 = Foo.find :all ... @assignN = Bar.find_for_user(logged_in_user) render :action => ... # or redirect_to ... else ... end end
Some details are left out here… And obviously, there will be cases where the Formula needs to be elaborated. There might even be a few cases where the Formula needs to be abandoned. But I think you’ll find it useful most of the time.