Clean up fat rails helpers

Question

Today, trying to DRY up some code, I extracted some duplicate File.exists? code used by several helper methods into a private method

def template_exists?(*template) and accidently monkey patched this already existing Rails helper method. This was a pretty clear indicator of a code smell, I didn't need need any the inherited methods, yet I inherit them. Besides, what was my refactored method doing in this helper at all?

So, this helper does way too much and hence violates SRP (Single Responsibility Principle). I feel that rails helpers are inherently difficult to keep within SRP. The helper I'm looking at is a superclass helper of other helpers, counting 300+ lines itself. It is part of a very complex form using javascript to master the interaction flow. The methods in the fat helper are short and neat, so it's not that awful, but without a doubt, it needs separation of concerns.

How should I go forth?

1. Separate the methods into many helpers?
2. Extracting the code inside the helpers methods into classes and delegate to them? Would you scope these classes (i.e. Mydomain::TemplateFinder)?
3. Separate the logic into modules and list them as includes at the top?
4. other approaches?

As I see, no 2 is safer wrt accidential monkeypatching. Maybe a combination?

Code examples and strong opinions appreciated!

Answer 1

Extracting helpers methods into classes (solution n°2)

ok to address this specific way to clean up helpers 1st I dug up this old railscast :

http://railscasts.com/episodes/101-refactoring-out-helper-object

At the time it inspired me to create a little tabbing system (working in one of my apps in conjunction with a state machine) :

module WorkflowHelper

  # takes the block  
  def workflow_for(model, opts={}, &block)
    yield Workflow.new(model, opts[:match], self)
    return false
  end

  class Workflow
    def initialize(model, current_url, view)
      @view = view
      @current_url = current_url
      @model = model
      @links = []
    end

    def link_to(text, url, opts = {})
      @links << url
      url = @model.new_record? ? "" : @view.url_for(url)
      @view.raw "<li class='#{active_class(url)}'>#{@view.link_to(text, url)}</li>"
    end

  private
    def active_class(url)
      'active' if @current_url.gsub(/(edit|new)/, "") == url.gsub(/(edit|new)/, "") ||
                 ( @model.new_record? && @links.size == 1 )
    end

  end #class Workflow
end

And my views go like this :

  -workflow_for @order, :match => request.path do |w|
    = w.link_to "✎ Create/Edit an Order", [:edit, :admin, @order]
    = w.link_to "√ Decide for Approval/Price", [:approve, :admin, @order]
    = w.link_to "✉ Notify User of Approval/Price", [:email, :admin, @order]
    = w.link_to "€ Create/Edit Order's Invoice", [:edit, :admin, @order, :invoice] 

As you see it's a nice way to encapsulate the logic in a class and have only one method in the helper/view space

Answer 2

1. You mean seperate large helpers into smaller helpers? Why not. I don't know your code too well, but you might want to consider outsourcing large code chunks into ./lib.
2. No. ;-) This sounds awfully complex.
3. Sounds complex too. Same suggestion as in 1.: ./lib. The modules in there are auto-loaded if you access them.
4. No

My suggestion is: Hesitate from using too many custom structures. If you have large helpers, ok, might be the case. Though I wonder if there is an explanation why that whole helper code is not in the Controller. I use helpers for small and simple methods that are used inside the template. Complex (Ruby-)logic should be put into the Controller. And if you really have such a complex Javascript app, why don't you write this complex stuff in Javascript? If it really has to be called from the template, this is the way to go. And probably makes you website a bit more dynamic and flexible.

Regarding monkey patching and namespace collisions: If you have class name, method names etc. that sound common, check out if they are defined. Google, grep or rails console for them.

Make sure you understand which code belongs to

• Controller: Fill variables with stuff, perform user actions (basically the computation behind your page)

• Helper: Help doing simple stuff like creating a fancy hyperlink

  def my_awesome_hyperlink url, text "Fancy Link to #{text}" end

• ./lib: More complex stuff that is used by more than one Controller and/or also used directly by other components like Cucumber step definitions

• inside the Template as Ruby Code: Super easy to read
• inside the Template (or ./public) as Javascript code: Matter of taste. But the more dynamic your app, the more likely code belongs in here.