Removing the @ Hack in Rails Controllers

A while back, I wrote an article that described a collection of non-conventional strategies that I use to make Rails controllers more maintainable. While I still think all those thoughts are good, there is one in particular that I like so much I think it should become the standard way to do things in Rails.

Towards that goal, I am writing this article to make a case for why we should change how data is loaded in controllers, shared across actions and communicated to the view.

The Rails Way

Before I present my case, I think it is important to understand the “Rails Way”, what is clever about it and where it is deficient. Hopefully, this will make it clear why my proposal is better.

A verbose (non-Rails) way of loading data and communicating that data to the view is:

def show
  user = User.find params[:id]
  render :show, locals: { user: user }
end

Luckily, the Rails developers realized that every action would end with something similar to the last line and it would only be more annoying to write the more data we needed to pass to the template. They came up with two clever solutions to reduce this boilerplate:

• The @ variable hack
• Implicit rendering

Lets see what our action looks like with these two conventions in place:

def show
  @user = User.find params[:id]
end

Implicit rendering renders the template named after the action. I love this convention and want to keep it. The @ variable hack is what I have a problem with.

The Hack

What is the @ variable hack? Instead of passing the variable to the view via some sort of hash they decided it would be better if we could just mark the variable.

How do we do that marking? Ruby itself does have two ways to mark variables. A variable can be prefixed with the $ or @ symbol.

The $ symbol is no good as Ruby would then make that value global. What about @? Normally this indicates the variable should be shared among all method invocations operating on the same instance. This is supported natively by Ruby and most object-oriented languages call them instance variables.

The Rails developers realized that instance variables don’t have much purpose in controllers. Conceptually (from the developer perspective) a controller is basically:

controller = UsersController.new request
response = controller.show

I realize internally it is more than that, but conceptually it is:

• create an instance
• call a single method
• throws away the object.

This is more functional in nature and less object-oriented.

Since our controller object doesn’t stay around for long, and we only call one method, instance variables don’t have much purpose in controllers. The Rails developers saw this and decided to co-opt the @ marker for another purpose.

Although they are still technically instance variables, Rails sees these variables and copies them to the view. This removes the need to specify a hash of variables to pass to the view.

I call this a hack because we are using a feature for an unintended purpose. That doesn’t mean I disapprove because it is a hack. It is a clever use of an unused feature to reduce boilerplate. My problem is not the hack itself, but in practice it just doesn’t satisfy all our needs.

The Problems

What are the problems with the @ variable hack? The problem is not in co-opting the @ marker, but in sharing loading patterns across actions. We often have actions that have similar data needs. For example:

def show
  @user = User.find params[:id]
end
def edit
  @user = User.find params[:id]
end

Now do that for update, destroy, and any other member actions also. It gets very repetitive. Rails has a solution via callbacks. Instead of the above we might have:

before_action(only: %i[show edit]) { @user = User.find params[:id] }
def show
end
def edit
end

There are problems with this solution:

• Targeting only certain actions with the only and expect options is error prone. I have often seen applications that are loading the wrong data because they didn’t maintain those lists well.
• It it difficult to see that the action is actually doing something. Just looking at the action itself it appears to be doing nothing.
• It might be hard to catch the callback in a large real world controller. Especially if the callback is defined in a parent class or included module!
• Source order becomes important as some before_action hooks might need data from other before_action hooks.

Just Use Methods

My solution is conceptually pretty simple and can be summed up as “just use methods”.

As we consider all our needs, my proposal will quickly build up a good bit of boilerplate. Boilerplate is what the @ hack is trying to remove so this might seem like a regression. Stick with me for a minute and see if you like the ideas. At the end, we are going to wrap up all that boilerplate in a small bit of meta-programming to make it concise while still keeping the advantages.

Define a Method

In a typical object-oriented system, if we want one component to be able to get data from another component what is the simplest solution? The first component sends a message to the second component who responds with the information. This is one of the core idea of object-oriented programming. Message passing via methods.

What if instead of trying to send data to the view, we invert it and have the view ask the controller for the data it needs. So the view is sending a message asking for the data which the controller then provides it in a response.

This might look like the following in the view:

<%= controller.user.name %>

In the controller we might have:

def user
  User.find params[:id]
end

This is just a public method; regular object oriented ideas. This method can be used by different templates. If the edit and show template both need the user, they can both call the same method. If the index method doesn’t need it, then it doesn’t call it. We don’t load data that we don’t need. No need to maintain a list of only and except in a callback definition.

Memoize

We might want to print out a bunch of user attributes and we don’t want the controller loading a new instance every time, so lets memoize the loading:

def user
  @user ||= User.find params[:id]
end

Note, I am using an instance variable, but I am using them for their real purpose (sharing the data between different method invocations on the same instance). This doesn’t cover the nil case. A more complete solution is:

def user
  return @user if defined? @user
  @user = User.find params[:id]
end

A Helper

Now this is conceptually no longer a hack, but it is more verbose in our template (also our controller but we will get to that later). We don’t want to prefix every variable access with controller.. What if we created a helper method to proxy to the controller:

module UsersHelper
  def user
    controller.user
  end
end

Now our template is just:

<%= user.name %>

This is nicer, but creating a helper method for every one of these data loading methods could get annoying. Luckily, Rails has a solution for us. We can call helper_method with any controller method and Rails will create the helper method for us. Now the data loading in our controller looks like:

def user
  return @user if defined? @user
  @user = User.find params[:id]
end
helper_method :user

Assignable

To better share loading behavior between actions, I have also found it helps for these methods to be assignable. Let us imagine we need some access control. We might defined the following:

def users
  return @users if defined? @users
  @users = policy_scope User.all
end
helper_method :users
def user
  return @user if defined? @user
  @user = users.find params[:id]
end
helper_method :user

I am using one of these data loading methods (users) to assist in the implementation of another method (user). Unlike the before_action callback, the source order doesn’t matter. Since we are just calling methods, we could define user first.

This all seems well and good, but what if we also want our index action to allow searching? We could define our index action like this:

def index
  @users = users.search params[:q]
end

Now we are back to using the @ hack. If we put the search in the users data loading method, then our show action will also end up searching due to our implementation of user. The general problem is that we want to share some of the data loading behavior between actions (the policy_scope) but not other aspects (the searching).

The solution is assignment. What if we define another method that allows an action to refine the data:

private
def users= assigned_users
  @users = assigned_users
end

Now our index action can be defined as:

def index
  self.users = users.search params[:q]
end

Due to our memoization using the same instance variable (here again we are using instance variables for their purpose to share data between method invocations on the same instance) when the index view calls users it will get a list with the policy scope and search applied. When the show action calls user it will just get the policy scope filtering and not the search.

I made this assignment method private because I only use it within the actions (or a before_action hook). I have never found a reason to use it in outside components (such as the view).

Testing

Another huge advantage of this proposal is testing. These methods are now public methods so we can call them in our test. Consider a test to see if the index action is really searching. Before we might have had something like this (using rspec syntax and factorybot):

it 'searches for the given query' do
  create :user, last_name: 'Doe'
  create :user, last_name: 'Jackson'
  get '/users', params: { q: 'Doe' }
  expect( response.body ).to have_content 'Doe'
  expect( response.body ).not_to have_content 'Jackson'
end

Here we are trying to imply the controller did the right thing by looking for keywords in the rendered template. What if some part of the page accidentally had the term Jackson for an unrelated reason. Our test would fail not because the functionality is not working, but just a false positive.

Let us look at the same test using our public method:

it 'searches for the given query' do
  expected = create :user, last_name: 'Doe'
  create :user, last_name: 'Jackson'
  get '/users', params: { q: 'Doe' }
  expect( controller.users ).to eq [expected]
end

Here our test is stronger. We are ensuring the desired record was found and that was the only record found. No side effect can accidentally cause a false positive.

Reducing the Boilerplate

You might have been following along and thinking that you like what I am suggesting (hopefully), but there is the nagging boilerplate issue. Our data loading methods are now in total:

def user
  return @user if defined? @user
  @user = User.find params[:id]
end
helper_method :user
private
def user= assign_user
  @user = assign_user
end

Ugh! Luckily, Ruby is really good at simplifying common patterns via meta-programming. The two things we really need to provide are:

• The name of the data variable
• How to load that variable

In an effort to provide some symmetry with rspec (so it is familiar to other developers) I have named my method let. Here is what the data loading looks like with the meta-programming in place:

let(:user) { User.find params[:id] }

Concise, but still with all the advantages of the boilerplate version. What does that meta-programming look like?

Drop this in your app/controllers/concern directory and extend it into ApplicationController and you’re set. It is just a tiny bit of code. No need for a big library. Don’t like the name let, then change it.

An Example

What does a controller look like with this construct in place?

I like to remove the trivial differences between creating and updating so I name the template form and don’t have dummy templates that just render a partial. But you don’t need to do that if you don’t want.

The let construct doesn’t mean you have to change anything else about your controllers. Code them the way you want. let just shares loading of data better across actions, between the controller and view and makes your testing easier.

Influence

I do want to acknowledge the influence of the decent_exposure library on my proposal. This library first exposed me (no pun intended) to the idea. What I don’t like about decent_exposure is the implicit loading. It hides too much and it to difficult to customize.

I considered using that library, but never using the implicit loading. When I realized I just needed a small bit of meta-programming vs a big library I decided the concern was a better route.