5

I apologize if this doesn't follow good question guidelines, but I hope it's well in class with How to Manage CSS Explosion and receives a similarly helpful response.

I'm familiar with some basic view prolixity mitigation strategies such as the following:

  • Use helpers where appropriate
  • Don't repeat yourself
  • Use partials and layouts

Feel free to suggest something if I'm missing some big idea in the above list.

Nevertheless, I still end up with having several dimensions/degrees of freedom in my view, causing a lot of if-then statements or at least ternary blocks. For instance, in something I'm currently messing with, I'm working on a header bar for a program where the view is called when three "big" variables:

  • Whether the user is admin
  • Whether the user is logged in
  • Whether the page being viewed belongs to the user or someone else

It ends up looking like this mess:

<% content_for :subheader do %>
  <div class="row">
    <% if @user %>
      <% if @user == current_user %>
        <%= link_to 'My programs', user_programs_path(current_user), :class => 'active' %>
      <% else %>
        <%= link_to "#{@user.username}'s programs", user_programs_path(@user), :class => 'active' %>
      <% end %>
      <%= link_to 'Browse all programs', programs_path %>
    <% else %>
      <% if current_user %>
        <%= link_to 'My programs', user_programs_path(current_user) %>
      <% end %>
      <%= link_to 'Browse all programs', programs_path, :class => 'active' %>
    <% end %>
    <%= link_to 'New Program', new_program_path, :class => 'admin' if current_user.admin? %>
  </div>
  <% if @regions %>
    <div class="row second">
      <%= link_to 'Regional program search', request.fullpath, :class => 'active' %>
    </div>
  <% end %>
<% end %>

Ugly. Readable and easily accessible, but ugly. Some suggestions?

Between experience and new technologies like LESS, I've become pretty good at slimming down my CSS files, but I'm still running into explosion issues with my MVC views.

Community
  • 1
  • 1
Steven
  • 17,796
  • 13
  • 66
  • 118
  • This is going to be one of those questions that nobody feels game enough to answer because it's such an opinionated question.. Am looking forward to hearing what people come up with though! – 2potatocakes Feb 17 '11 at 00:36
  • I honestly wish there were a good way to "disarm" this question. If you (as well as anybody else) have any good ideas, feel free to edit the question. – Steven Feb 17 '11 at 00:57

2 Answers2

6

I would use helpers and model definitions to dry up your code:

class User
  def possesive
    self == current_user ? 'My' : "#{username}'s"
  end
end

module ...Helper
  def user_program_link user
    if user
      link_to "#{user.possesive} programs", user_programs_path(user), :class => 'active'
    elsif current_user
      link_to 'My programs', user_programs_path(current_user)
    end
  end
end

You can then simplify all the if statements for the user_program_path calls to this:

<%= user_program_link @user %>

Which would reduce your view code to:

<% content_for :subheader do %>
  <div class="row">
    <%= user_program_link @user %>
    <% if @user %>
      <%= link_to 'Browse all programs', programs_path %>
    <% else %>
      <%= link_to 'Browse all programs', programs_path, :class => 'active' %>
    <% end %>
    <%= link_to 'New Program', new_program_path, :class => 'admin' if current_user.admin? %>
  </div>
  <% if @regions %>
    <div class="row second">
      <%= link_to 'Regional program search', request.fullpath, :class => 'active' %>
    </div>
  <% end %>
<% end %>

Continue this process to DRY up the rest of your code as well.

Pan Thomakos
  • 34,082
  • 9
  • 88
  • 85
  • In the link helper, `if` condition should be `if user and user != current_user`. – Harish Shetty Feb 17 '11 at 03:48
  • 1
    No, the way I have it is correct, because when there is a user a class of 'active' is required and the possessive method takes care of the proper text formatting. – Pan Thomakos Feb 17 '11 at 03:53
0

Rewrite the view code as follows:

<% content_for :subheader do %>
  <div class="row">
    <% if @user ||  current_user %>      
      <%= link_to ((current_user == @user or @user.nil?) ? "My programs" : 
                    "#{@user.username}'s programs"), 
                  user_programs_path(@user || current_user), 
                  :class => 'active' %>
    <% end %>
    <%= link_to 'Browse all programs', programs_path, 
          :class => (@user ? '' : 'active') %>
    <%= link_to 'New Program', new_program_path, :class => 'admin' if current_user.admin? %>
  </div>
  <% if @regions %>
    <div class="row second">
      <%= link_to 'Regional program search', request.fullpath, :class => 'active' %>
    </div>
  <% end %>
<% end %>
Harish Shetty
  • 64,083
  • 21
  • 152
  • 198