public
Created — forked from jonsmock/0_the_explanation.txt

Oh the conditionals!

  • Download Gist
0_the_explanation.md
Markdown

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.

1_the_model.rb
Ruby
1 2 3 4 5 6 7 8
class Employee < ActiveRecord::Base
include EmployeeStatus
def hired_on; end
def received_employee_handbook?; end
def scheduled_by; end
def last_worked_on; end
end
2_the_mixin.rb
Ruby
1 2 3 4 5 6 7
module EmployeeStatus
def recently_hired?; end
def scheduled?; end
def points_are_concerning?; end
def deliquent_points; end
end
3_the_supporting_class.rb
Ruby
1 2 3 4 5 6 7
class Roster
attr_accessible :employees
def initialize(employees)
@employees = employees
end
end
4_the_controller.rb
Ruby
1 2 3 4 5
class RosterController < ApplicationController
def index
@roster = Roster.new(Employees.morning_shift)
end
end
5_the_view.html.erb
HTML+ERB
1 2 3 4 5 6 7 8 9
<ul>
<% @roster.employees.each do |employee| %>
<li>
<h2><%= employee.full_name %></h2>
 
<%= present(employee).description %>
</li>
<% end %>
</ul>
6_employee_presenter.rb
Ruby
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49
# This assumes a base presenter setup similar to RailsCasts episode #287: http://railscasts.com/episodes/287-presenters-from-scratch
class EmployeePresenter < BasePresenter
presents :employee
 
def description
if employee.recently_hired?
recently_hired_description + handbook_description
elsif employee.points_are_concerning?
discipline_description
elsif employee.scheduled?
scheduled_description
else
unscheduled_description
end
end
 
private
 
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))
else
content_tag(:p, "#{hired_on} but still needs to be scheduled.") + link_to('Schedule now', new_employee_shift_path(employee))
end
end
 
def handbook_description
if employee.received_employee_handbook?
"".html_safe
else
content_tag(:p, "Make sure they have a copy of the handbook!")
end
end
 
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.")
end
 
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)))
end
 
def unscheduled_description
content_tag(:p, "This employee is in good standing.") + link_to('View Shifts', employee_path(employee))
end
end

Please sign in to comment on this gist.

Something went wrong with that request. Please try again.