Premature Extraction October 2nd, 2007

You should break common HTML fragments into seperate files to keep things DRY. These reusable fragments are called partials.

If you want to render an 'example' partial and pass it the '@variable' variable as 'local_name':

<%= render :partial => 'example', :locals => {:local_name => @variable } %>

Which is pretty "verbose" syntax, but isn't too bad. You can even hide it behind a helper. Today I see this:

<% with_options({:locals => {:listing => listing}}) do |products| %>
  <%= products.render :partial => 'listings/products/services' if listing.services %>
  <%= products.render :partial => 'listings/products/brands' if listing.brands %>
<% end %>

And each partial is basically:

<h3>Brands</h3>
<p>
  <%= with_newlines_as_breaks listing.brands %>
</p>

with_options is a helpful method mixed into #Object by ActiveSupport. It saves space when you need to pass a big hash to several methods. Jamis Buck has a great writeup on it. Here, it's a sign you're being too clever.

This template doesn't need partials. The HTML is never reused. The partials only add obfuscation. When you load the template the first time, first you have to figure out what's going on with the partial. Then, you have to jump to the partial. This is simpler:

<%- if listing.services -%>
  <h3>Services</h3>
  <p>
    <%= with_newlines_as_breaks listing.services %>
  </p>
<%- end -%>
<%- if listing.brands -%>
  <h3>Brands</h3>
  <p>
    <%= with_newlines_as_breaks listing.brands %>
  </p>
<%- end -%>

A good rule of thumb in refactoring is "three strikes and you're out." So if we had another attribute, say "hours," we'd refactor it to:

<%- %w[services brands hours].each do |attribute| -%>
  <%- if listing.send(attribute) -%>
    <h3><%= attribute.capitalize %></h3>
    <p>
        <%= with_newlines_as_breaks listing.send(attribute) %>
    </p>
  <%- end -%>
<%- end -%>