Use memoization to clean up your controllers

I'll admit it. I'm not a fan of callback filters in Rails. It's one of those things, like spring and turbolinks, that I remove when I encounter them. I mean no offense to the people that wrote those things. I'm sure they have their places, but I am suspicious of anything that saves me a few imperceptible seconds here and there yet winds up costing a very tangible hour every few months because of some hidden detail. Once bitten, twice shy.

Callback filters are those before_action lines at the top of controller classes that run some method before the called method... or after it... or before and after as in the case of around_action.

 

Memoization is a way of creating a method that returns the value of a variable if it has one, or sets the value if it doesn't and then returns it. In ruby, it can be done with the ||= assignment operator.
@dollars ||= @cents / 100.0
...says ,"If @dollars has a value, return it. Otherwise, set @dollars to @cents divided by 100 and then return it."

The most obvious problem with callback filters is how unobvious they are. If you are debugging, you have to know of and remember about their existence even though you will never encounter it; you can't simply trace through code line by line because they do their business out-of-band. That requires you and your classes to have knowledge of how unrelated classes behave, sometimes they are not even in the inheritance tree. One could argue this is antithetical to the the single responsibility and encapsulation principles of object oriented design, but let's not go there.

Because you can ≠ You should

There are good use-cases for filters, like "mark fantasy football league standings stale after a new game is saved." (Even then I can think of better ways to handle that.)

However, "Set this variable before these five methods," is not a valid use-case. It's lazy, inconsiderate, and not doing anything that can't be done better some other way. If your mantra is I clean up code by hiding it in a callback then you are violating the Principle of Least Astonishment (POLA) and let's do go there. It's sweeping a mess under the rug, which is not really "cleaning up."

On the other hand, you don't want to have an assignment statement for that variable in each of those five methods either. That would violate the DRY principle.

Having your cake, and skinny controllers too

For the past three years, I have been substituting callbacks and class variables with memoization to clean up code and improve readability while also adhering to both POLA and DRY. It works in controllers very well, but in models just as nicely. It's one of those tools, like Tell Don't Ask, that really improved my code quality once I adopted it.

An example

Try this. Let's take the venerable Article controller as created by Rails scaffold generator.

class ArticlesController < ApplicationController
  before_action :set_article, only: [:show, :edit, :update, :destroy]

  # GET /articles
  # GET /articles.json
  def index
    @articles = Article.all
  end

  # GET /articles/1
  # GET /articles/1.json
  def show
  end

  # GET /articles/new
  def new
    @article = Article.new
  end

  # GET /articles/1/edit
  def edit
  end

  # POST /articles
  # POST /articles.json
  def create
    @article = Article.new(article_params)

    respond_to do |format|
      if @article.save
        format.html { redirect_to @article, notice: 'Article was successfully created.' }
        format.json { render :show, status: :created, location: @article }
      else
        format.html { render :new }
        format.json { render json: @article.errors, status: :unprocessable_entity }
      end
    end
  end

  # PATCH/PUT /articles/1
  # PATCH/PUT /articles/1.json
  def update
    respond_to do |format|
      if @article.update(article_params)
        format.html { redirect_to @article, notice: 'Article was successfully updated.' }
        format.json { render :show, status: :ok, location: @article }
      else
        format.html { render :edit }
        format.json { render json: @article.errors, status: :unprocessable_entity }
      end
    end
  end

  # DELETE /articles/1
  # DELETE /articles/1.json
  def destroy
    @article.destroy
    respond_to do |format|
      format.html { redirect_to articles_url, notice: 'Article was successfully destroyed.' }
      format.json { head :no_content }
    end
  end

  private
    # Use callbacks to share common setup or constraints between actions.
    def set_article
      @article = Article.find(params[:id])
    end

    # Only allow a list of trusted parameters through.
    def article_params
      params.require(:article).permit(:title, :body)
    end
end

I'm not going to make this into a tutorial and go through it step-by-step. Instead, let's cut to the chase, shall we? This version replaces the callback with @article memoized into an article method.

class ArticlesController < ApplicationController
  def index
    @articles = Article.all
  end

  def show
    article
  end

  def new
    @article = Article.new
  end

  def edit
    article
  end

  def create
    @article = Article.new(article_params)

    respond_to do |format|
      if article.save
        format.html { redirect_to article, notice: 'Article was successfully created.' }
        format.json { render :show, status: :created, location: article }
      else
        format.html { render :new }
        format.json { render json: article.errors, status: :unprocessable_entity }
      end
    end
  end

  def update
    respond_to do |format|
      if article.update(article_params)
        format.html { redirect_to article, notice: 'Article was successfully updated.' }
        format.json { render :show, status: :ok, location: article }
      else
        format.html { render :edit }
        format.json { render json: article.errors, status: :unprocessable_entity }
      end
    end
  end

  def destroy
    article.destroy
    respond_to do |format|
      format.html { redirect_to articles_url, notice: 'Article was successfully destroyed.' }
      format.json { head :no_content }
    end
  end

  private
    # Do NOT use callbacks to share common setup or constraints between actions.
    def article
      @article ||= Article.find(params[:id])
    end

    def article_params
      params.require(:article).permit(:title, :body)
    end
end

All but two of the class variables were replaced with calls to the article. Even after setting @article in the create method, I immediately go back to using memoization for consistency.

The benefits will come when someone tries to debug this class. But it will really start to show as this class grows. Let's say you need to add a non-RESTful action to "approve" an article. That might look something like this:

  def approve
    respond_to do |format|
      if article.approve
        format.html { redirect_to article, notice: 'Article was approved.' }
        format.json { render :show, status: :ok, location: article }
      else
        format.html { render :edit }
        format.json { render json: article.errors, status: :unprocessable_entity }
      end
    end
  end

I didn't need to edit the method array in callback definition because there isn't one. If you have to maintain a piece of code so another piece can run, that violates a core principle of Rails, Convention Over Configuration.

Now, whenever I type @something in a controller, I ask myself if "something" shouldn't be memoized.

Depending on the complexity of the controller, I might also memoize article_params into an parms method. If you are running through the params hash for filtering and sorting calling require, permit, and merge is redundant and therefore inefficient. "Repeating Yourself" doesn't just mean typing the same thing over and over, it also means doing the same thing over and over. Memoizing means that those methods are only called once.

Summary

If I had my druthers, Rails scaffold generators wouldn't create callbacks, not because I use generators, but because new developers do and I feel callback filters send them down the wrong road.