Want to see the full-length video right now for free?Sign In with GitHub for Free Access
Rails 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
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
as well as we're inheriting from it (and possibly others). When we couple this
with implicit methods invoked via
before_filters, the complexity can grow
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
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!
A common example of introducing implicit global state is the use of a
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
@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
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
@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
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
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
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
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
extracted to a
$ 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
destroy actions, but we feel this minor duplication is absolutely worth
it for the added clarity and explicitness it provides.
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
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
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
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.
To recap, here are the collected recommendations for encouraging encapsulation and avoiding global state in your Rails applications:
before_filters, especially in parent controllers like