Striving Towards Maintainable Controllers

Eric Anderson
9 min readMar 21, 2017

--

Controllers have always been an area I’ve never been happy with in Rails. Let’s look at a “recommended” controller based on the scaffolding:

Let’s be honest. This isn’t that bad.

In fact, it used to be a lot worse when it was considered good practice to make all your actions respond to various formats (to overload your app implementation to also be your API implementation). Luckily that was removed.

What I still don’t like:

  • The strong params is ugly and error prone to maintain. Especially on complex forms. I pine for the days that we just used raw params. Sure it caused a security hole, but this is Ruby. Can’t we have convenience and security?
  • There is subtle repetition between many of the actions, but small differences still exist. This makes us stuck with lots of different but similar code.
  • Three actions share some data loading but it’s not clear from looking at the action itself. The callback is registered at the top and the loading is defined at the bottom, both far away from the actions using the data. The action itself makes no reference to the fact that we are even using this data in some cases.

There have been efforts before to make controllers better but none have ever had any staying power. Lets look at where others have failed to see if we can come up with a better way forward.

Strides Within Rails

Early versions of rails had a macro to dynamically provide the repetitive implementation. It usage was simple:

class WidgetsController < ApplicationController
scaffold :widget
end

This macros expanded out into all the common actions and templates. You could override with a solid implementation of each action as your app developed, eventually removing the scaffold macro. There was just one problem, people used it in production even though the purpose was just for scaffolding. Because of this, it was removed to discourage misuse.

This lead to the natural question. Perhaps we can have something like this but meant to be used for more than just scaffolding?

Strides Outside Of Rails

make_resourceful was an early attempt at this goal. It encapsulated the common “resource” behavior, had hooks and callbacks for customization and wrapped it in a DSL to make it friendly to manipulate. It generally looks something like:

class WidgetsController < ApplicationController
make_resourceful do
actions :all
end
end

Are we done? Do we now have nice easy controllers? Not really as controllers rarely conform completely to a common behavior. You start using the hooks provided, and then using more, then more. Pretty soon, it’s a mess of hooks duck-taped together.

Initially people thought that DSL was the problem. Perhaps that magic is making it too difficult to work with? After make_resourceful, resource_controller was born which still had some DSL but relied heavier on callbacks. It also fully supported the resource concept by responding to different formats. The basic controller looked like:

class PostsController < ApplicationController
resource_controller
end

Despite the goal of being better, it still lacked the utopia desired. Eventually the controller also ended up being a mess of hooks duck-taped together.

Yet another attempt was made via inherited_resources. This project took out all the DSL and tried to rely on inheritance rather than callbacks under the idea that too many callbacks were the cause of the missing utopia. It’s basic controller looks like:

class ProjectsController < InheritedResources::Base
end

Hiding Too Much

The author of inherited_resources later nailed the reason all these attempts to make controllers easier to write have failed when he stopped recommending people use inherited_resources. They hide too much of the logic. Your controller become scraps of callbacks and overrides with the real meat hidden away in a 3rd party library.

He recommends instead using responders. For a while, responders were even partially included in Rails core before it is determined that it also hides too much and was pulled back out (also a single action returning multiple formats has thankfully fallen out of favor).

A New Hope

In hopes that we are not doomed to the annoyance of writing controllers I continued searching for a better way and came across decent_exposure which offered a possibility. Here is a basic decent_exposure controller:

class ThingsController < ApplicationController
expose :things, ->{ Thing.all }
expose :thing

def create
if thing.save
redirect_to thing_path(thing)
else
render :new
end
end

def update
if thing.update(thing_params)
redirect_to thing_path(thing)
else
render :edit
end
end

def destroy
thing.destroy
redirect_to things_path
end

private

def thing_params
params.require(:thing).permit(:foo, :bar)
end
end

Decent exposure stops trying to hide the control flow logic of controller. Instead it just provides a method to share the loading of data across similar methods. The expose declarations are similar to writing a instance method, only supercharged with a few things. I.E. conceptually:

expose :things, -> { Thing.all }

is similar to:

def things
Thing.all
end

By sharing the loading across actions, the actions themselves got simplified enough that they could just be explicitly defined. Since they are just calling a method you don’t have the disconnect present with before_action callbacks.

In addition to sharing data loading across actions, it also provides a more official interface for views and tests to interact through rather than the instance variable hack that Rails encourages.

I indicated that expose is a super-charged version of defining a method. What exactly is supercharged about it?

  • The value is memoize so you can call it as often as you want in handling an action without a performance hit.
  • The method is registered as a helper method. This means your view can just use things rather than controller.things. Conceptually it is the same but it’s a nice convenience.
  • It is actually assignable. This means you can share loading across actions but then still have a specific action further refine the data. This makes it more like attr_accessor rather than just defining a method.

There is one final bit of supercharging in decent_exposure and really the only thing I don’t like about it. It has implicit loading. You will notice in the above example we don’t actually define the loading of the instance object. This is because decent_exposure assumes common loading patterns if the block is not given.

While concise this also means we have moved back to hiding stuff. The loading of the data is hidden somewhere in the decent_exposure library. Although decent_exposure provides ways to customize this loading it still becomes hidden logic that is difficult to modify.

My Strategy

Perhaps I use decent_exposure and just always provide a block? That seems fairly reasonable but that’s a lot of unused code. Could we possibly have something like decent exposure, but without the magic loading? Also what about the subtle repetition and annoying strong params. It does nothing about those?

To solve this I have come up with my own way of doing controllers that I think accomplishes the goal of making controller less annoying to write, less repetitive but still explicit enough to easily customize and understand. First let me show you the example basic controller:

class WidgetsController < ApplicationController
let(:widgets) { Widget.all }
let(:widget) { widgets.find_or_initialize_by id: params[:id] }
defs :new, :edit do
render 'form'
end
defs :create, :update do
if widget.update params[:widget]
redirect_to edit_widgets_path(widget),notice: 'Saved widget'
else
render 'form'
end
end
def destroy
widget.destroy
redirect_to widgets_path, notice: 'Widget removed'
end
end

The next few sections will break this down a bit but even if you don’t understand everything, it is clear:

  • All data loading and control flow is explicit. Although I use constructs such as let and defs to be concise, nothing is hidden.
  • It doesn’t have the repetitiveness you see in the scaffolded controller.
  • It lacks the a lot of boilerplate like strong params, registering loading callbacks, etc.

If the above controller looks appealing continue further where I break down each construct used. Finally I’ll show a controller from a real application that is not “basic” (i.e. uses authorization, searching, etc) and show it still holds up. Real controllers is where I felt the previous callback approach falls down and I think these constructs hold up well.

Lettable

The first thing you probably notice is the let declarations. These are conceptually inspired by decent_exposure but without the loading magic. I choose to use the keyword let as I like it’s symmetry with the spec syntax. The entire implementation is:

This is placed in the app/controllers/concern directory and in my ApplicationController I extend with the module so all controllers inherit the construct. Looking at the implementation you will see:

  • It is memoizing the value allowing repeated access with no extra cost.
  • It registers as a helper method for view convenience.
  • It provides a setter which is private to the controller (not accessible in the view). In our final real world example you will see how this is used.

I choose to memoize it to a instance variable named after the exposed method. This provides a nice compatibility with the traditional rails setup. There are parts of Rails and 3rd party components that assume @widget will exist. This way we get memoization and compatibility in one stroke.

Another nice thing is since the data has an official public interface it makes testing that the right data is loaded easier. Just call the exposed data directly on the controller in your tests rather than trying to imply the loaded data by reading the views.

MultiDef

The second thing you might notice is the defs contruct. My goal is to get rid of repetition by making the methods so similar that they can use the same implementation. defs simply defines multiple methods with the same implementation:

Like before, I drop this in controller/concerns and extend my ApplicationController. If you look at the scaffolded controller in Rails it creates annoying differences that doesn’t need to be there. Some of these differences:

  • Different notice message. We can just say it was “saved” rather than “created” or “updated”?
  • Different templates. Creating and updating use the same form template. Rarely are these templates any different. In fact they usually just render to a _form partial. The difference is usually just the label of the submit button at the bottom (again use the “save” terminology rather than “create” or “update”) and perhaps the heading (again an unnecessary difference).
  • Different save methods. Why does create assign the params when creating the object then call save while update calls update to both assign params and save. Can both methods use the same mechanism?

By removing these unnecessary differences we can just have them use the sample implementation. The defs construct makes it easy to define that shared implementation.

The only problem I gained is that I can no longer rely on the default template working for new and edit. Fortunately my defs construct can solve that easily as well by making them both render form.

I don’t go to extraordinary efforts to use defs. If the actions need to be different, then I don’t use the construct. But for the common case it is nice.

Signed Forms

The final thing to address is the dropping of the strong params. Did I just add a security hole? No, as I’m actually using a 3rd party gem called signed_form.

Rather than define my form fields in my template, then list them again in the controller, signed_form simply has the form tell me what fields are valid. It signs this list to avoid tampering. This gives me all the security of strong params (in fact it’s build on top of strong params) but the simplicity of how things used to be in Rails.

I’ve used signed_form on several projects now. There are a few edge cases that could be sanded better but they can be worked around easily. Also with further contributions by others I’m sure we can remove some of those edge cases.

Real Controller

How well does my controller strategy hold up in real controllers? Here is one taken from real code:

Some things that make this controller more real:

  • It is authorizing the records (using Pundit). Easy to add to the let declarations since the loading is all explicit. Since the loading is shared across actions we lock everything down in one place.
  • The agents exposure is just defining a general scope that agent builds on. We then further refine that scope in the index action by filtering for active records, applying a search, sorting and paginating the results. The fact that the exposed variable can be assigned makes this sort of specialization in an action easy.
  • This entire controller is in a namespace but I didn’t have to do anything special since all the control flow (route helpers) and data loading are explicit.
  • I have other app-specific controller concepts such as ListParams and a filter helper easily integrated. Trivial to do again because everything is explicit.
  • In this app, I don’t need a destroy action. Since everything is explicitly defined I just don’t define it, unlike ResourceController where I have to un-define the method defined for me.

Next Steps

We are not done yet. I’m sure in a year or two we’ll have come up with even newer ways to make controllers better. I consider how I do controllers a work in progress. But I think it is a lot better than what scaffolding gave me.

--

--