开发者

How can I refactor out needing so many for-loops in rails?

开发者 https://www.devze.com 2022-12-30 22:21 出处:网络
I need help refactoring this multi-loop thing.Here is what I have: Campaign has_many Contacts Campaign also has many Models which are templates: (Email, Call, and Letter).

I need help refactoring this multi-loop thing. Here is what I have:

Campaign has_many Contacts

Campaign also has many Models which are templates: (Email, Call, and Letter).

Because I am looking for overdue on each, I created an array called Event which I'd like to loop through that contains ['email', 'call', 'letter'].

I need a list of all the开发者_JAVA技巧 Emails, Calls and Letters that are "overdue" for every Contact that belongs to a Campaign. Overdue is determined by a from_today method which looks at the date the Contact was entered in the system and the number of days that needs to pass for any given Event. from_today() outputs the number of days from today that the Event should be done for a given Contact.

Here is what I've done, it works for all Emails in a Campaign across all contacts. I was going to try to create another each do loop to change the class names.

Wasn't sure where to begin: named_scope, push some things into a method, etcetera, or -- minimum to be able to dynamically change the class names so at least it loops three times across the different events rather than repeating the code three times:

<% @campaigns.each do |campaign| %>
   <h2><%= link_to campaign.name, campaign %></h2>

   <% @events.each do |event| %>
       <%= event %>
       <% for email in campaign.emails %>
          <h4><%= link_to email.title, email  %> <%= email.days %> days</h4>

          <% for contact in campaign.contacts.find(:all, :order => "date_entered ASC" ) %>
             <% if (from_today(contact, email.days) < 0) %>
                <% if show_status(contact, email) == 'no status'%>
                    <p> <%= full_name(contact) %> 
                        is <%= from_today(contact,email.days).abs%> days overdue:
                        <%= do_event(contact, email) %>
                    </p>
                <% end %>
             <% end %>
          <% end %>
       <% end %>
     <% end %>
<% end %>


Just to add to Patrick's answer, I would also use the :collection option of render to simplify this a bit further, e.g. have a partial _contact.html.erb to render each contact:

<% if (from_today(contact, email.days) < 0) %>
    <% if show_status(contact, email) == 'no status'%>
      <p> <%= full_name(contact) %> 
          is <%= from_today(contact,email.days).abs%> days overdue:
          <%= do_event(contact, email) %>
      </p>
    <% end %>
  <% end %>
<% end %>

and then render the contacts collection with

= render :partial => "contact", :collection => @contacts

I also wouldn't do a find in the view, instead I would setup all the variables in the controller, and probably move all the conditional code into a helper. It's preferable to keep as much logic as possible out of the views.


I'd put the output for each resource into a partial, like so:

<% @campaigns.each do |campaign| %>
  <h2><%= link_to campaign.name, campaign %></h2>
  <%= render 'events', :events => campaign.events %>
<% end %>

then in app/views/campaigns/_events.html.erb

<% events.each do |event| %>
  <%= event %>
  <%= render 'emails', :emails => event.emails %>
<% end %>

then in app/views/campaigns/_emails.html.erb

<% emails.each do |email| %>
  <h4><%= link_to email.title, email  %> <%= email.days %> days</h4>
  <%= render 'contacts', :contacts => email.contacts.all(:order => "date_entered ASC", :email => email) %>
<% end %>

then in app/views/campaigns/_contacts.html.erb

<% contacts.each do |contact| %>
  <% if (from_today(contact, email.days) < 0) %>
    <% if show_status(contact, email) == 'no status'%>
      <p> <%= full_name(contact) %> 
          is <%= from_today(contact,email.days).abs%> days overdue:
          <%= do_event(contact, email) %>
      </p>
    <% end %>
  <% end %>
<% end %>
0

精彩评论

暂无评论...
验证码 换一张
取 消

关注公众号