back

4 steps to a cleaner rails view

Refactoring in Ruby can lead to dramatically cleaner code.

Here’s a quick example that Abe and I worked on over the course of a minute or two of working on a drag and drop feature together. I obfuscated it a bit for privacy.

We started with:

1
2
3
4
5
6
7
8
9
10
11
12
13
14

# edit.html.erb

<% @model.draggables.each do |draggable| %>
  <li id="<%=dom_id(draggable) %>">
  <% if draggable.name %>
    <div class="handle"></div>
    <b><%= draggable.name %></b>
    <div class="delete"></div>
  <% else %>
     <div class="handle"></div>
     <b><%= draggable.associated_model.another_associated_model.name %></b>
     <div class="delete"></div>
  </li>
<% end %>

We quickly changed this to:

1
2
3
4
5

# draggable.rb
def to_s
    return name if name
    associated_model.another_associated_model.name
 end

with:

1
2
3
4
5
6

# edit.html.erb
<% @model.draggables.each do |draggable| %>
  <li id="<%=dom_id(draggable) %>">
     <h2><%=h draggable.to_s %></h2>
   </li>
<% end %>

And then remembered that <%= calls to_s automatically, so we could instead have:

1
2
3
4
5
6
7
8
9
10

# edit.html.erb
<% @model.draggables.each do |draggable| %>

  <li id="<%=dom_id(draggable) %>">
     <h2>
       <%=h draggable %>
      </h2>
   </li>

<% end %>

Then, we saw that the ternary operator could do what we wanted in the model in one clear line, so we did:

1
2
3
4

# draggable.rb
def to_s
  name ? name :  associated_model.another_associated_model.name
end

And after putting it all on one line, it became clear that it was simplest to just do:

1
2
3
4

# draggable.rb
def to_s
  name || associated_model.another_associated_model.name
end

While each step didn’t give us a huge gain, the final result is about half as much code as the initial try, and also much clearer.

blog comments powered by Disqus

August 26, 2009

Looking for more? Go to the archives.

Contact: github message me.