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.