← archive

When magic works against you

November 22, 2020

When I first encountered this concern in the Rails app that I was working on, I was initially impressed. It felt like magic. Like a cool trick to avoid unnecessary complexity.

This concern enabled using let in Rails controllers. It inverts the relationship between controllers and views, so instead of controllers providing data for the views, it makes views ask for data from controllers. This post shows the reasoning behind it, and I will refer to it as “original post” in this article.

This is the example from the original post:

class WidgetsController < ApplicationController
  let(:widgets) { Widget.all }
  let(:widget) { widgets.find_or_initialize_by id: params[:id] }

  def new
    render :form
  end

  def edit
    render :form
  end

  def create
    save
  end

  def update
    save
  end

  def destroy
    widget.destroy
    redirect_to widgets_url,
                notice: "Widget was successfully destroyed."
  end

  private

  def save
    if widget.update secure_params
      redirect_to widget,
                  notice: "Widget was successfully saved."
    else
      render :form
    end
  end

  def secure_params
    params.require(:widget).permit :name
  end
end

With this, the templates would simply use widget:

<%= widget.name %>

However, there are a lot of problems with this approach.

1. It’s implicit

There’s a reason why ActiveRecord callbacks are not a great idea and why you should prefer not passing instance variables to your views. The Zen of Python famously generalizes this rule as “explicit is better than implicit”.

Code that relies too much on magic is too difficult to understand and therefore maintain. I would rather have duplication that is visible and easy to understand than DRY magic that is hard to understand.

2. It’s a fake local

The original post sees passing instance variables to views as an improvement of explicitly passing locals. (I strongly disagree because I find views with instance variables hard to maintain.)

let is more implicit than using instance variables. It suffers from the same problems as using instance variables since it uses instance variables under the hood. It makes views too dependent on their controllers. Partials become difficult to move around.

let adds another problem and makes it more difficult for the reader to understand that this is, in fact, a memoized instance variable.

It’s a memoized instance variable masquerading as a local.

So, just adding this concern into your application controller, as the author suggests in his original post, makes all of your views more difficult to understand.

Seeing:

<%= user %>

In any of your views makes it difficult to track down. Is it a local or an instance variable?

3. It’s easy to make mistakes

If you’re not careful, you can easily open a can of worms. One example is using let inside the application controller. Here’s an example:

class ApplicationController < ActionController::Base
  include Lettable

  let(:user) do
    User.find(params[:user_id]) if params[:user_id].present?
  end
end

Good luck tracking where user comes from in hundreds of your views or controllers.

* * *

So, by introducing the Lettable concern, you end up with somewhat cleaner controllers and views that become hard to understand. You avoid the issue of accidentally loading data for actions that don’t need it, as mentioned in the original post, but it makes your life much harder when it comes to maintenance.

Beware of the long term consequences of including cool tricks in your codebase. They might temporarily solve smaller problems but sometimes cause even bigger problems. What starts as magic can end tragic.

Want to talk more about this or any other topic? Email me. I welcome every email.