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