2

I'm a newbie at Rails and I'm having trouble wrapping my head around refactoring logic from views. Let's say I have a simple Post model. In the index view, I want specific content to be displayed if there are posts or not. Basically, if there are any posts, display this specific content or else this other content.

Here is my index.html.erb view for Posts:

<div class="content">
 <% if @posts.any? %>
 <table>
     <thead>
       <tr>
         <th>Title</th>
         <th>Content</th>
       </tr>
     </thead>
     <tbody>
       <% @posts.each do |post| %>
         <tr>
           <td><%= post.title %></td>
           <td><%= post.content %></td>              
         </tr>
       <% end %>
     </tbody>
   </table>
 <% else %>
 <p>There are no posts!</p>
 <% end %>
</div>

Now, the way I refactored this was by creating a couple of helpers and partials like so:

posts_helper.rb (which renders the partials according to the if logic):

module PostsHelper

 def posts_any
  if @posts.any?
    render 'this_content'
  else
    render 'this_other_content'
  end
 end

end

In the partials, I just used the exact content in the if else statement.

_this_content.html.erb partial:

<table>
   <thead>
     <tr>
       <th>Title</th>
       <th>Content</th>
     </tr>
   </thead>
   <tbody>
     <% @posts.each do |post| %>
       <tr>
         <td><%= post.title %></td>
         <td><%= post.content %></td>              
       </tr>
     <% end %>
   </tbody>
 </table>

_this_other_content.html.erb partial:

<p>There are no posts!</p>

Finally, the refactored index.html.erb (which would call the helper method):

<div class="content">
 <%= posts_any %>
</div>

The problem is, I'm just not convinced that this is the correct Rails way of refactoring. If any of you could shed some light on this, I would highly appreciate it! Thanks!

mmcdevi1
  • 21
  • 2

1 Answers1

6

You're doing it right, and better than many people I know. :)

A few minor adjustments...

I would move the render from the helper to the erb, and just use the helper to return the right name of what to render.

Your erb code and helper code:

<%= posts_any %>

def posts_any
  if @posts.any?
    render 'this_content'
  else
    render 'this_other_content'
  end
end

I suggest:

<%= render posts_any %>

def posts_any
  @posts.any? ? 'this_content' : 'this_other_content'
end

Next, I personally like to render a collection using a partial.

Yours:

 <% @posts.each do |post| %>

I suggest:

<%= render partial: "post", collection: @posts %>

And in the comment below, user kyledecot suggests even terser:

<%= render @posts %>

Then create the file _post.html.erb like this:

<tr>
  <td><%= post.title %></td>
  <td><%= post.content %></td>              
</tr>

Some developers think that it's overkill to render a collection using a partial, in the case where the partial is not used anywhere else.

I personally think it's helpful, and especially useful when a project has multiple coders some of whom may be changing the table row data results.

joelparkerhenderson
  • 34,808
  • 19
  • 98
  • 119