Create a gist now

Instantly share code, notes, and snippets.

What would you like to do?
Oh the conditionals!

This example is not my real app, but it's modeled pretty closely to it.

My mom schedules a couple hundred employees at a local warehouse. In their system, a worker can earn "points" for being late or missing shifts. Here we have a sort of summary screen for my mom, who may need to follow up with new employees or discipline employees with concerning levels of points.

Questions that arise as I code something like this:

  • Which objects deserve presenters?

Any object with a decent amount of complexity in the view. I prefer to refactor presenters out. Therefore they grow out of complexity and true need rather than up-front design.

  • Should I allow HTML in my presenters?

I usually do. The only reason I see this being an issue is when you need to present different formats (json). If you reach this complexity then it may be worth splitting one presenter off into multiple formatted presenters, one for each format.

  • Do partials help or do they actually hide complexity that should be

I generally use partials for two reasons: 1) for removing duplication in the view. 2) for representing HTML content behind complex logic.

  • Is this amount of logic ok in a view, if it's tested? (Views are essentially methods, right?)

I think so, but maybe borderline. It looks like the core logic is nicely factored into the model, so the if statements are each a simple, single clause. One thing that bothers me is the elsif after the long if block. The only other thing is the closing </p> tag inside the if statement. Ick.

See the code below for how I would refactor this into a presenter based on RailsCasts episode 287

Of course, those kinds of questions make a 2 hour feature take a day...

Don't worry about the questions so much. Do the feature in two hours and then refactor that as you see fit. The key is to stay disciplined in refactoring which allows you to make ugly code at first just to do the simplest thing that could possibly work.

class Employee < ActiveRecord::Base
include EmployeeStatus
def hired_on; end
def received_employee_handbook?; end
def scheduled_by; end
def last_worked_on; end
module EmployeeStatus
def recently_hired?; end
def scheduled?; end
def points_are_concerning?; end
def deliquent_points; end
class Roster
attr_accessible :employees
def initialize(employees)
@employees = employees
class RosterController < ApplicationController
def index
@roster =
<% @roster.employees.each do |employee| %>
<h2><%= employee.full_name %></h2>
<%= present(employee).description %>
<% end %>
# This assumes a base presenter setup similar to RailsCasts episode #287:
class EmployeePresenter < BasePresenter
presents :employee
def description
if employee.recently_hired?
recently_hired_description + handbook_description
elsif employee.points_are_concerning?
elsif employee.scheduled?
def recently_hired_description
hired_on = "This employee was hired on #{employee.hired_on}"
if employee.scheduled?
content_tag(:p, "#{hired_on} and was scheduled by #{employee.scheduled_by}!") + link_to('View Shifts', employee_path(employee))
content_tag(:p, "#{hired_on} but still needs to be scheduled.") + link_to('Schedule now', new_employee_shift_path(employee))
def handbook_description
if employee.received_employee_handbook?
content_tag(:p, "Make sure they have a copy of the handbook!")
def discipline_description
content_tag(:p, "This employee needs disciplinary action. They currently have #{pluralize(employee.deliquent_points, 'point')}") +
content_tag(:p, "Please consult management before scheduling more shifts.")
def scheduled_description
content_tag(:p, "This employee is in good standing. They last worked on #{employee.last_worked_on}.") +
link_to('View Shifts', employee_path(employee)))
def unscheduled_description
content_tag(:p, "This employee is in good standing.") + link_to('View Shifts', employee_path(employee))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment