Can this be refactored?

Hi --

Below is code for a user referral form displayed only if several conditions are met.

1) can this be refactored?

   <% if session[:person_id] %>      <% unless session[:person_id] == @person.id %>        <% unless @referrals.any? {|referral| referral.giver_id == session[:person_id]} %>

You could conceivably push this down into a model. However...

         <%= render :partial => 'referrals/form' %>        <% end %>      <% end %>    <% else %>      <p><em>Please log in to add a referral for <%= @person.full_name %></em></p>    <% end %>

2) is there anything wrong with having this in a view? Alternatives?

I would tend to advocate putting this logic in the controller. Accessing the session directly in the view always feels a bit transgressive to me -- no real harm in it, but session always suggests controller logic, since the controller rather than the view is in charge of the session. This is of course more a concern with writing to the session than reading from it, but I still tend to think of it as the controller's job.

I'm thinking of something like (give or take any flaws in the logic I've introduced):

   def my_action      ...      @referee = true unless                session[:person_id] == @person.id or                @referrals.any? {|r| r.giver_id == session[:person_id] }

   end

(You don't need the explicit assignment to 'true', but I like the way it reads :slight_smile:

and then in the view:

   <% if @referee %>      <%= render :partial => 'referrals/form'    <% else %>      <p> ...    <% end %>

David