Looping Through Multiple Arrays in Ruby

Question

I have multiple arrays of instances of ActiveRecord subclass Item that I need to loop through an print in accordance to earliest event. In this case, I need to print print out payment and maintenance dates as follows:

Item A maintenance required in 5 days
Item B payment required in 6 days
Item A payment required in 7 days
Item B maintenance required in 8 days

I currently have two queries for finding maintenance and payment items (non-exclusive query) and something like the following to output them:

<%- item_p = nil -%>
<%- item_m = nil -%>

<%- loop do -%>
  <% item_p ||= @items_p.shift %>
  <% item_m ||= @items_m.shift %>

  <%- if item_p.nil? and item_m.nil? then break -%>
  <%- elsif item_p and (item_m.nil? or item_p.paymt < item_m.maint) then -%>
    <%= item_p.name %> payment required in ...
  <%- elsif item_m and (item_p.nil? or item_m.maint < item_p.paymt) then -%>
    <%= item_m.name %> maintenance required in ...
  <%- end -%>
<%- end -%>

Any way to cleanup the readability of the above (ugly) code?

Answer 1

Embrace duck-typing and make sure that your objects are polymorphic. You want your payment items to be comparable with maintenance items, in order to sort them.

So, suppose you have a Payment and a Maintenance class:

module Due
  include Comparable

  # Compare this object with another. Used for sorting.
  def <=>(other)
    self.due <=> other.due
  end
end

class Payment < ActiveRecord::Base
  include Due

  alias_method :due, :payment

  def action
    "#{name} requires payment"
  end
end

class Maintenance < ActiveRecord::Base
  include Due

  alias_method :due, :maintenance

  def action
    "#{name} requires maintenance"
  end
end

See how we create an action, due and <=> method in both classes? We also took care to include the Ruby built-in module Comparable. This allows us to do the following:

# Assuming 'payment' and 'maintenance' are date fields...
a = Payment.new :payment => 3.days.from_now
b = Maintenance.new :maintenance => 2.days.from_now
[a, b].sort
#=> [b, a]

The view then becomes as simple as:

<% (@payment_items + @maintenance_items).sort.each do |item| %>
  <%= item.action %> in <%= distance_of_time_in_words_to_now(item.due) %><br/>
<% end %>

I'm sure I haven't got the details of your implementation right, but I hope this gives you an idea of how to approach your problem.

Answer 2

This is quick and dirty (i.e. not optimized):

# In your controller:
@items = @items_p.map{ |item| {:item => item, :days => item.paymt, :description => "payment"} } 
@items += @items_m.map{ |item| {:item => item, :days => item.maint, :description => "maintenance"} }
@items = @items.sort_by{ |item| item[:day] }

# In your view:
<% @items.each do |item| %>
  <%= item[:item].name %> <%= item[:description] %> required in <%= item[:days] %> days
<% end %>

Answer 3

You're doing way too much in your view. Really you should figure out all of this in the controller and pass through a cleaned up structure that you can iterate over for display purposes.

As an example:

length = [ @items_p.length, @items_m.length ].sort.last

@messages = [ ]

length.times do |i|
  item_p = @items_p[i]
  item_m = @items_m[i]

  if (item_p and (item_m and item_p.paymt < item_m.maint) or !item_m)
    @messages << "#{item_p.name} payment required in ..."
  elsif (item_m and (item_p and item_m.maint < item_p.paymt) or !item_p)
    @messages << "#{item_m.name} maintenance required in ..."
  end
end

You would then iterate over @messages as required.

The real problem here is that you haven't structured these objects for this sort of thing strategically speaking. It would be nice if you had a single method for the due date instead of having to differentiate between paymt and maint according to the type. Likewise, it would be better if both of these were paired up into an Array instead of supplied separately.

If you had them in [ p, m ] pairs you could iterate much more simply:

items.each do |pair|
  first_due = pair.compact.sort_by(&:due).first
  @messages << "#{first_due.name} #{first_due.action} required in ..."
end

The action method would return payment or maintenance as required.