Skip to content

Instantly share code, notes, and snippets.

@josephbridgwaterrowe
Last active April 6, 2016 17:56
Show Gist options
  • Save josephbridgwaterrowe/0a15a9255ad497264d67b906bef9f7c3 to your computer and use it in GitHub Desktop.
Save josephbridgwaterrowe/0a15a9255ad497264d67b906bef9f7c3 to your computer and use it in GitHub Desktop.
Two different controller patterns

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.

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
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
@josephbridgwaterrowe
Copy link
Author

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

👍

@frank-west-iii
Copy link

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.

@brianriley
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment