Too many private methods!

I don’t like bloated views, I guess nobody does. Views can easily become the worst nightmare of any application, when care is not properly taken.

I was working on an application with lots of search forms spread throughout the views. Those forms were not backed by ActiveRecord, so I had to use pure Rails form helpers, like text_field_tag. When you use these kinds of helpers too much in your views they quickly become a mess.

I eventually got tired of building forms that way, and felt the need to code a little class which builds those forms in an elegant and straightforward manner.

My first step was to think about the form builder at a higher level. How would I like to build my form?

<%= search_form for: @model do |f| %>
  <%= f.text_field 'Search term', :by_term %>
  <%= f.select 'Status' :by_status, collection %>

  <%= f.submit 'Search' %>
<% end %>

That looks clean and good. The by_term and by_status parameters point to ActiveRecord scopes related to the @model object. There is some code in the backend responsible for securely performing searches using those scopes - I will not show the code here due to brevity purposes.

Besides building the form fields, here are some additional requirements:

Each field must accept an options hash, just like the existing Rails form helpers.
It must automatically fill in each field’s value after submit, gracefully keeping up with the form state.
You are not required to pass a model, it is optional. The model can be used to help building the current form state, but the implementation must know other sources where to search for the input’s value attribute.

Coding our form builder

Now we need to implement the code to make that interface work. We proceed by creating a Rails helper method named search_form:

module FormHelper
  def search_form(options = {}, &block)
    form_options = options.delete(:form_options) || {}
    form_options[:method] ||= :get

    form_for :search_form, form_options do
      options[:context] ||= self
      SearchForm.new(options, &block)
    end
  end
end

This helper method builds the HTML for our input fields, wrapping it inside a form tag. As a next step we will build the SearchForm class:

class SearchForm
  def initialize(**options)
    @view              = options.fetch(:context)
    @model             = options.fetch(:for, options[:model])
    @input_base_class  = options.fetch(:input_base_class,  'form-control')
    @submit_base_class = options.fetch(:submit_base_class, 'btn btn-default')

    yield self
  end

  def text_field(field_label, field_id, **options)
    options[:class] = css_class(options)

    field_value = find_value(options, field_id)
    field_html  = @view.text_field_tag(field_id, field_value, options)

    inline_labeled_field(field_html, field_label)
  end

  def select(field_label, field_id, collection, **options)
    options[:class] ||= css_class(options)

    value      = find_value(options, field_id)
    collection = @view.options_for_select(collection, value)
    field_html = @view.select_tag(field_id, collection, options)

    inline_labeled_field(field_html, field_label)
  end

  def submit(title, **options)
    options[:class] ||= @submit_base_class

    @view.submit_tag(title, options)
  end

  private

  def inline_labeled_field(field_html, field_label)
    return field_html if field_label.nil?

    @view.content_tag(:label) do
      "#{field_label}: #{field_html}".html_safe
    end
  end

  def css_class(options)
    [@input_base_class].push(options[:class]).compact.join(' ')
  end

  def find_value(options, field_id)
    options.delete(:value)         ||
    find_value_in_filter(field_id) ||
    find_value_in_request(field_id)
  end

  def find_value_in_filter(field_id)
    @model.try :get_filter_param, field_id
  end

  def find_value_in_request(field_id)
    @view.params[field_id]
  end
end

This is the code we need to make our form work, also taking into account our additional requirements. I will not include the tests here, so we can keep our focus narrowed on the main subject.

Note that we could have handled the main form tag there, instead of in the helper, but that’s OK for now.

How it works

The constructor accepts a context option. That context will most likely be the view context, which gives us access to regular Rails helpers. Take a look at the search_form helper method to see how we are handling the context.
The public methods are responsible for building the form inputs that we need. They use raw Rails form helpers.
The methods text_field and select both find the input’s value after submit. First they try to find it in the options hash; if it’s not there, they look into the model, using a get_filter_param method. I will not show it here, but the only thing you need to understand is that the model keeps track of the last performed search, and accepts values from outside of the request params - that way, if we wish to initialize the filter with default search params we can do so, and the form will still know how to build its state. Finally, if it doesn’t find the value in the model, it looks for it in the request params.
Both methods return the result of inline_labeled_field, which itself returns a complete field together with its label. We only need the inline layout for now.

There are lots of things going on there. There are many designs to approach our problem, but this is the simplest we could get at first.

Too many private methods!

Notice that we have 5 private methods. Too many private methods can be a sign our class is doing too much, and gives us a clue that we can abstract it even further. In Ruby we have the awesome feature of nested classes. We can build these artifacts to lessen our burden, and make our code cleaner and more organized.

The find_value_in_filter and find_value_in_request methods could be condensed into the main find_value method, but we want to make our intent as clear as possible, effectively giving all things a name to ease the life of people who read our code, and also for ourselves - so we prefer to keep both methods.

But we can do even better: we can abstract the three methods into their own nested class inside SearchForm. We choose to name the new class FieldValueFinder:

class FieldValueFinder
  def initialize(model, view)
    @model, @view = model, view
  end

  def find(options, field_id)
    options.delete(:value)   ||
    find_in_filter(field_id) ||
    find_in_request(field_id)
  end

  private

  def find_in_filter(field_id)
    @model.try :get_filter_param, field_id
  end

  def find_in_request(field_id)
    @view.params[field_id]
  end
end

This class’ responsibility is narrow and focused. We can easily understand what it does in its context, and its dependencies are clear and explicit. It makes our SearchForm class easier to reason about, and more compliant, although not completely, with the single responsibility principle (SRP). This is the final SearchForm class for now:

class SearchForm
  def initialize(**options)
    @view              = options[:context]
    @input_base_class  = options.fetch(:input_base_class,  'form-control')
    @submit_base_class = options.fetch(:submit_base_class, 'btn btn-default')

    model         = options.fetch(:for, options[:model])
    @value_finder = FieldValueFinder.new(model, @view)

    yield self
  end

  def text_field(field_label, field_id, **options)
    options[:class] = css_class(options)

    field_value = @value_finder.find(options, field_id)
    field_html  = @view.text_field_tag(field_id, field_value, options)

    inline_labeled_field(field_html, field_label)
  end

  def select(field_label, field_id, collection, **options)
    options[:class] ||= css_class(options)

    value      = @value_finder.find(options, field_id)
    collection = @view.options_for_select(collection, value)
    field_html = @view.select_tag(field_id, collection, options)

    inline_labeled_field(field_html, field_label)
  end

  def submit(title, **options)
    options[:class] ||= @submit_class

    @view.submit_tag(title, options)
  end

  private

  def inline_labeled_field(field_html, field_label)
    return field_html if field_label.nil?

    @view.content_tag(:label) do
      "#{field_label}: #{field_html}".html_safe
    end
  end

  def css_class(options)
    [@input_class].push(options[:class]).compact.join(' ')
  end

  class FieldValueFinder
    def initialize(model, view)
      @model, @view = model, view
    end

    def find(options, field_id)
      options.delete(:value)   ||
      find_in_filter(field_id) ||
      find_in_request(field_id)
    end

    private

    def find_in_filter(field_id)
      @model.try :get_filter_param, field_id
    end

    def find_in_request(field_id)
      @view.params[field_id]
    end
  end
end

A quick note about nested classes: although this one is inside the private section of our main class, that private declaration doesn’t have any effect with classes. Anyway, we are signaling that the child class is private, and clients can’t rely on its API. We can also mark the class with :nodoc:, to make our intent even clearer.

Wrapup

Our class is now ready to grow, and is less of a monolithic monster. Although it is a bit longer, the concepts are clearer. It is by no means perfect, but that’s exactly the point: you should not try to refactor everything right away, not until you really understand what’s going on. Sometimes early refactoring leads to the wrong abstraction.

There are things to watch out for: In the future, if we ever need to render a form input with a disposition other than “inline”, our first approach would be to create one more private method, for example, block_labeled_field.

This is a great opportunity to refactor out that bit, moving the code to another class in order to handle the layout concern. Another good reason to do that is you will likely need to refactor the existing inline_labeled_field method to share functionality with the block_labeled_field method. You will not want to bloat your SearchForm class with that, will you?

Thiago A. Silva

Too many private methods!

Coding our form builder

How it works

Too many private methods!

Wrapup

You might also enjoy (View all posts)