Want to see the full-length video right now for free?
Sign In with GitHub for Free AccessRails makes many of our day to day tasks much easier, but sometimes this ease comes at a cost. Consistently across applications we see a lack of encapsulation and the shared global state of controllers and views to be one of the biggest sources of complexity and bugs. In this episode we dive into the causes, and solutions to this complexity in hopes of building easier to maintain Rails apps.
In each of controllers, views, and helpers we have access to each of
session
, params
, flash
, cookies
, as well as any instance variables set
in the controllers.
To expand on the instance variable point, while we are only running a single
action in a single controller at a given time, it turns out that at a minimum
we also pick up any instance variables defined in the ApplicationController
as well as we're inheriting from it (and possibly others). When we couple this
with implicit methods invoked via before_filter
s, the complexity can grow
very quickly.
When we start out on new applications, these sorts of coupling may seem like
nothing to worry much over. It's very easy to justify one or two special cases
where params
are accessed in views or helpers, or an instance variable is
accessed in a partial. The danger is that these sorts of state leakages tend
to compound, and tend to do so quickly, causing a few special cases to
suddenly be the norm and pervasive.
Further, it's worth noting that this sort of coupling is very resistant to refactoring and often when an application has become entangled in these ways, the most common reality is that it will stay that way and the team will just come to accept the complexity and difficulty in making changes as facts of life. This is certainly a case where an ounce of prevention is worth a pound of cure!
before_filter
A common example of introducing implicit global state is the use of a
before_filter
in ApplicationController
to assign an instance variable.
class ApplicationController < ActionController::Base
include Authentication
protect_from_forgery with: :exception
before_filter :find_organization
def find_organization
if current_user
@organization = current_user.organization
end
end
end
In this application, a user is part of an organization, so logically we might
want to always have the @organization
instance variable set to allow us to
display the org name in the header, and having this automated via a
before_filter
lookup seems might nice and efficient.
Unfortunately this can quickly lead to problems if we have a concrete
controller, for instance OrganizationsController
which ends up overriding
the @organization
instance variable. It might be easy to forget about the
@organization
instance variable as it is automatically assigned to in the
ApplicationController
, out of sight, out of mind. This can quickly lead to
displaying the wrong data to the user, or much more seriously, allowing users
to act on organizations they are not part of.
One step we can take to help clean this up is to make the user's organization lookup exposed via a helper method, rather than as an instance variable. This makes it a bit more explicit, and protects from overriding via the child controllers.
class ApplicationController < ActionController::Base
# ...
def current_user_organization
if current_user
@_current_user_organization ||= current_user.organization
end
end
helper_method :current_user_organization
end
In the initial implementation, the _header
partial used the @organization
instance variable directly. This is almost always something we want to avoid
as it couples our partial to the implicit global state of the controller the
current action was rendered through, and it makes it harder to understand what
value the @organization
might have when rendering the partial.
Instead, we almost always want to explicitly render a partial by passing in any objects we need as locals:
# app/views/layouts/application.html.erb
- <%= render "header" %>
+ <%= render "header", user: current_user, organization: current_user_organization %>
# app/views/application/_header.html.erb
- Welcome, <%= current_user.name %> from <%= @organization.name %>
+ Welcome, <%= user.name %> from <%= organization.name %>
Here at thoughtbot we've recently been exploring a number of alternative frameworks for building web applications including Elixir+Phoenix, React, Elm, end even more recent versions of Ember, and a common theme is a move towards a more functional approach to defining UIs.
When a view is pulling from any number of objects provided by implicit global
state (session
, flash
, instance variables, etc.), it becomes increasingly
difficult to look at that view in isolation and understand how it will behave.
That said, if we can transition to only rendering our view with data
explicitly passed in (as locals
in the case of Rails partials), then that
view becomes very easy to understand and test. Big wins for long term
maintainability!
Taking this to the logical extreme in Rails, we currently have a pull request
open in the thoughtbot guides repo that suggests passing in all data via
locals
even when rendering top level views: Advocate using locals to render
views.
In hopes of keeping this practical, here are some of the high level rules we suggest as a way to mitigate coupling within the controller and view layers of your application:
This one is pretty hard and fast. We should always explicitly pass data into a
partial as locals
, and not rely on an instance variable from the parent
context. This decouples our partial allowing for easier reuse throughout the
application, and makes the partial easier to understand and test in isolation.
Whenever possible, and this is likely the vast majority of the time, avoid
assigning to instance variables via before_filter
methods. This hides the
assignment making it less clear what views rely on that data, and makes it
possible to accidentally override or mutate that data in child contexts.
Instead, expose the same lookup logic via a helper method and either use that method in your views, or use that method to assign to an instance variable in the specific action where that data is needed.
Note: currently the rails scaffold
generator will generate a controller
with the default model lookup for show
, edit
, update
, and destroy
extracted to a before_action
:
$ bin/rails generate scaffold Project
will produce the following controller:
class ProjectsController < ApplicationController
before_action :set_project, only: [:show, :edit, :update, :destroy]
# GET /projects/1
def show
end
# ... Other actions omitted
private
# Use callbacks to share common setup or constraints between actions.
def set_project
@project = Project.find(params[:id])
end
end
We recommend avoiding this and instead consider having a private method for model lookup, but keeping the instance variable assignment local to the action currently being executed:
class ProjectsController < ApplicationController
def show
@project = find_project
end
private
def find_project
Project.find(params[:id])
end
end
the lookup / assignment line above will be duplicated in the edit
, update
,
and destroy
actions, but we feel this minor duplication is absolutely worth
it for the added clarity and explicitness it provides.
session
The session in Rails is way to associate additional state with a user,
typically by storing it in a cookie, to make that state available when
rendering a view. At a minimum we'll likely end up identifying a signed in
user via the session, but often we'll end up reaching for the session
to
store other ad-hoc data. This is another sneaky source of global state.
For instance, in our example we want to show the current user a list of other
users who have been added to the organization since last the current user
viewed the list. We can use the session
as a quick and easy way to store the
state of when the current user last viewed:
# app/views/users/_list.html.erb
<ul>
<% @organization.users.where("created_at > ?", Time.parse(session[:last_viewed])) do |user| %>
<li>
<%= user.email %>
</li>
<% end %>
</ul>
<% session[:last_viewed] = Time.current %>
Here we're first breaking the rule of never referencing instance variables in
partials, but we're also using (and mutating) the session
. This global state
again makes the view harder to understand and test as we now have to deal with
all of the values that could be in the session
.
Likewise, since session is stored on a given device, we're left with a situation where users can see out of sync data across various machines. Instead, if the data is important to your application, it likely deserves persistence in the database.
An extension of the explicitness described above is to embrace the dependency inversion principle (the "D" in SOLID). Rather than hard coding references to our dependencies in our classes, we instead invert control and write our classes such that all dependencies are passed in as arguments. For example:
class SubscriberCounter
def count
User.where.not(subscription: nil)
end
end
In the above class, we have hard coded our dependency on the User
class by
directly referencing the User
constant. Instead, we could write our class
as:
class SubscriberCounter
def initialize(users:)
@users = users
end
def count
users.where.not(subscription: nil)
end
end
In this case, we've inverted control by requiring that the users collection be
passed in as an argument to the constructor (user Ruby 2.0 required keyword
argument). Now if we want to operate on just users in the current
organization rather than all users, the SubscriberCounter
will not need to
change. We've isolated it from changes in its dependencies by passing them in,
rather than hard coding them.
Joe Ferris took this to the extreme in the upcase-exercises app by creating the payload gem to take advantage of dependency inversion within the app.
To recap, here are the collected recommendations for encouraging encapsulation and avoiding global state in your Rails applications:
before_action
/before_filter
s,
especially in parent controllers like ApplicationController
session
, params
, cookies
, and flashs
in views