Formatting September 20th, 2007

You can tell a lot about a person by their formatting.

<li class="first">
<% if listing.rateable? %><%= logged_link_to :rate_it, listing, "Rate it", new_rating_url(listing.listing_id), :onclick => 'return signInOrPopup(this.href);' %><% else %>Rate it<% end -%></li><%if listing.has_ratings? %><li><%= logged_link_to :read_reviews, listing, "Read Reviews", more_info_reviews_url(listing) %></li><% else %><li>Read Reviews</li><% end %>

This tells me: "WATCH OUT!"

Step 1: What the hell is going on?

With a little cleaning, we can survey the mess.

<li class="first">
    <% if listing.rateable? %>
        <%= logged_link_to :rate_it, listing, "Rate it", new_rating_url(listing.listing_id), :onclick => 'return signInOrPopup(this.href);' %>
    <% else %>
        Rate it
    <% end %>
</li>
<% if listing.has_ratings? %>
    <li>
        <%= logged_link_to :read_reviews, listing, "Read Reviews", more_info_reviews_url(listing) %>
    </li>
<% else %>
    <li>Read Reviews</li>
<% end %>

That's better.

It's a simple pattern: "If that link applies to the record, make it a link. Otherwise, make it plain text." There should be a helper for that. Like link_to_if.

link_to_if(listing.has_ratings?, "Read Reviews", more_info_reviews_url(listing))

Except, we're using our own helper for logging purposes called "logged_link_to." Writing logged_link_to_if is a sign something is wrong with our overall logging system, and we should be overriding link_to with an alias_method_chain, but that's a project for another day. I just need something so I can sleep at night.

<li class="first">
  <%=
    if listing.rateable?
      logged_link_to(:rate_it, listing,
        "Rate it", new_rating_url(listing.listing_id),
        :onclick => 'return signInOrPopup(this.href);')
    else
      'Rate it'
    end %>
</li>
<li>
  <%=
    if listing.has_ratings? 
      logged_link_to(:read_reviews, listing,
      "Read Reviews", more_info_reviews_url(listing))
    else
      'Read Reviews'
    end
   %>
</li>