Two different controller patterns

_rails_controller_comparison.md

Rails Controller Comparison

This Gist compares two different patterns to writing your controllers. My personal preference is orders_first_controller.rb, please feel free to comment with pros and cons of both patterns.

Notes

For this example we are using locals in both examples since instance variables encourage a leaky controller and views.

Machine comparisons were performed with flog and flay

Both scored a 0 on flay.

orders_first_controller.rb

class OrdersFirstController < ApplicationController
  def create
    render_new unless new_order.save

    redirect_to orders_path
  end

  def edit
    render_edit
  end

  def index
    render locals: { orders: orders }
  end

  def new
    render_new
  end

  def show
    render locals: { order: order }
  end

  def update
    render_edit unless order.update_attributes(order_params)

    redirect_to orders_path
  end

  def render_new
    render :new, locals: { locations: locations, order: new_order }
  end

  def render_edit
    render :edit, locals: { locations: locations, order: order }
  end

  private

  def new_order
    @order ||= Order.new(order_params)
  end

  def order
    @order ||= Order.find(params[:id])
  end

  def orders
    @orders ||= Order.all
  end

  def order_params
    return {} unless params[:order]

    params
      .require(:order)
      .permit(:destination_location_id,
              :requested_delivery_date,
              :time_slot)
  end

  def locations
    @locations ||= Location.all
  end
end

# flog output
#    41.4: flog total
#     3.0: flog/method average
#
#     6.8: OrdersFirstController#update     app/controllers/orders_first_controller.rb:24
#     5.9: OrdersFirstController#order_params app/controllers/orders_first_controller.rb:52
#     5.6: OrdersFirstController#create     app/controllers/orders_first_controller.rb:2
#     3.7: OrdersFirstController#order      app/controllers/orders_first_controller.rb:44
#     3.4: OrdersFirstController#render_edit app/controllers/orders_first_controller.rb:34

orders_second_controller.rb

class OrdersSecondController < ApplicationController
  def create
    order = Order.new(order_params)

    if order.save
      redirect_to orders_path
    else
      render :new, locals: { order: order, locations: locations }
    end
  end

  def edit
    order = Order.find(params[:id])

    render locals: { order: order, locations: locations }
  end

  def index
    render locals: { orders: Order.all }
  end

  def new
    order = Order.new

    render locals: { order: order, locations: locations }
  end

  def show
    order = Order.find(params[:id])

    render locals: { order: order }
  end

  def update
    order = Order.find(params[:id])

    if order.update_attributes(order_params)
      redirect_to orders_path
    else
      render :edit, locals: { order: order, locations: locations }
    end
  end

  private

  def order_params
    return {} unless params[:order]

    params
      .require(:order)
      .permit(:destination_location_id,
              :requested_delivery_date,
              :time_slot)
  end

  def locations
    @locations ||= Location.all
  end
end

# flog output
#    43.3: flog total
#     4.8: flog/method average
#
#    10.7: OrdersSecondController#update    app/controllers/orders_second_controller.rb:34
#     8.1: OrdersSecondController#create    app/controllers/orders_second_controller.rb:2
#     5.9: OrdersSecondController#edit      app/controllers/orders_second_controller.rb:12
#     5.9: OrdersSecondController#order_params app/controllers/orders_second_controller.rb:46

Props to @frank-west-iii for breaking out def order (with some unnecessary branching) into def new_order and def order.

👍

There are a couple of reasons that I like this implementation.

The first is dealing with global state:
With ivars you are encouraging global state usage throughout the controller, views, partial views and helpers. Global state is a slippery slope and in my opinion is best avoided. With an ivar of @order there is no way to control the usage of that ivar in a partial or in a helper. If you start down that path it is difficult to find where these ivars are being created. There are several instances where ivars are being created in before_action methods within the controller or the application controller itself. With explicit locals you can be sure that you know where they are being passed in and can easily find the declaration. You may even go as far as coming up with a convention such as @_order where an underscore denotes that this is "private" and you should not see it anywhere outside the controller.

The second point is about render_* methods:
For simpler controllers that pass a single object to the view, you may not see the benefit of this approach immediately. However when you start passing multiple items to the view, having a way to ensure that you are passing everything that the view needs to render properly is much less error-prone. I can't remember the number of times that I have seen errors where a collection was not initialized in the create or update action that was all of a sudden needed because the new or edit needed to be re-rendered due to a validation error. This approach eliminates that error altogether.

class OrdersController < ApplicationController
  def create
    order = new_order(order_params: order_params)

    render_new(order: order) and return unless order.save

    redirect_to orders_path
  end

  def edit
    render_edit
  end

  def index
    render locals: { orders: orders }
  end

  def new
    render_new
  end

  def show
    render locals: { order: order }
  end

  def update
    render_edit and return unless order.update_attributes(order_params)

    redirect_to orders_path
  end

  private

  def order_params
    params
      .require(:order)
      .permit(:destination_location_id,
              :requested_delivery_date,
              :time_slot)
  end

  def locations
    @_locations ||= Location.all
  end

  def new_order(order_params: {})
    @_order ||= Order.new(order_params)
  end

  def order
    @_order ||= Order.find(params[:id])
  end

  def orders
    @_orders ||= Order.all
  end

  def render_new(order: new_order)
    render :new, locals: { order: order, locations: locations }
  end

  def render_edit
    render :edit, locals: { order: order, locations: locations }
  end
end