Cleaning Up Rails Helper Methods With A Helper Class: Good Idea, Bad Idea, Or ‘Meh’?


I found myself writing a very ugly helper method in my rails ApplicationHelper module:

def render_program_dashboard(patient_program)
    key = patient_program.program.key
    content_for :css do
      stylesheet_link_tag "#{key}/patient_program"
    end

    dashboard_instance = nil
    dashboard_class_name = "#{key.to_s.classify}::ProgramDashboard"

    begin
      dashboard_class = dashboard_class_name.constantize
      dashboard_instance = dashboard_class.new(patient_program)
    rescue
      Rails.logger.info "Dashboard Class Not Found: #{dashboard_class_name}"
    end

    render :partial => "#{key}/program/dashboard", :locals => { :dashboard => dashboard_instance }
  end

This chunk of code uses some conventions to include a stylesheet and a partial view in the view that calls it, and it also instantiates a class that the partial uses to populate itself with data. … But I don’t like this code living inside of a helper method. It’s very specific to one part of my app, not the app in general. It’s also a little large and ugly and could use some cleaning up, splitting into separate methods, etc… all of which I’m reluctant to do in a helper method, because the helper module would start to get really ugly really quickly.

My answer to this was to move the code into a class on it’s own, codifying the concept that was being skirted around with this helper method. I created a ProgramDashboard class that contains this code, and cleaned up a tiny bit of it along the way. In order to use the render and content_for methods in the class, though, I needed access to the view that was being rendered. To solve this, I pass the view into the class along with the patient_program. The code ends up looking like this:

class ProgramDashboard
  def self.render(view, patient_program)
    key = patient_program.program.key

    view.content_for :css do
      view.stylesheet_link_tag "#{key}/patient_program"
    end

    dashboard = get_dashboard key, patient_program

    view.render :partial => "#{key}/program/dashboard", :locals => { :dashboard => dashboard }
  end

  private 

  def self.get_dashboard(key, patient_program)
    dashboard_instance = nil
    dashboard_class_name = "#{key.to_s.classify}::ProgramDashboard"

    begin
      dashboard_class = dashboard_class_name.constantize
      dashboard_instance = dashboard_class.new(patient_program)
    rescue
      Rails.logger.info "Dashboard Class Not Found: #{dashboard_class_name}"
    end

    dashboard_instance 
  end
end

I like this solution because it encapsulates a specific set of functionality for a specific part of the app better than a helper method does. It keeps my code cleaner, over-all, and keeps me from bleeding a bunch of abstractions that most of my app don’t need via helper methods.

I know there are things I can do to clean up the ProgramDashboard class. What I’m really interested in, though, is whether or not this is a good idea, a bad idea, or just “meh” – who cares…

Let me know what you think. Do you do things like this? Is there something better that I can do? Is this something that works in ‘the right circumstances’ and I just happened to stumble across a scenario that supports it? I’m interested in all feedback on this.

Cells: Partial Controllers And Views For Rails 3