views:

29

answers:

2

Hi, everyone: I am also open to just straight-up refactoring what I'm finding to be pretty repetitive, but to give a baseline of how it's working....

I have for every contact a Campaign, which has_many of three types of Models: Email, Call, and Letter.

When an Email (Call or Letter) has been executed for a specific contact, I have a Contact_Email(_or_Call_or_Letter) which belongs to both the Contact and the Model (Email_or_Call_or_Letter).

Each Contact_Email for example pairing has a :date_sent attribute. So does each Contact_Call and Contact_Letter.

How do I find the latest of all of them?

Here is the code I wrote that can find the latest Email and my finding retyping similar code for Call and Letter, but then stuck on how to do a .max on all of them:

  def last_email(contact)
    #get campaign the contact belongs to
    @campaign = Campaign.find_by_id(contact.campaign_id)

    @last_email = ContactEmail.find(:last, 
                        :conditions => "contact_id = #{contact.id}",
                        :order => "date_sent DESC")

    @last_call = ContactCall.find(:last, 
                        :conditions => "contact_id = #{contact.id}",
                        :order => "date_sent DESC")

    @last_letter = ContactLetter.find(:last, 
                        :conditions => "contact_id = #{contact.id}",
                        :order => "date_sent DESC")

    # how do I get the latest of all of these to display?

    @email_template = Email.find_by_id(@last_email.email_id)

    if @last_email.nil?
      return "no email sent"
    else
      return @last_email.date_sent.to_s(:long) + link_to('email was sent', @email_template)
    end
  end

Question 1: With what I have, how can I find effectively @last_event given I can find the last Email, last Call, and last Letter for every contact?

Question 2: How can I remove the repetitive code that I have to write for each Model?

A: 

For question 1:

Just do

@last_event = [@last_letter, @last_email, @last_call].sort_by{|m| m.date_sent}.first

For question 2:

Well this is more interesting. This kind of depends on how exactly do your models look, but you might want to consider Single Table Inheritance for this type of scenario.

Jakub Hampl
what happens if one of these instances (for example, @last_letter) has no value because no letter was ever sent?
Angela
+1  A: 

Do you have has_many associations setup in Contact referring to the other models? Something like:

class Contact < ActiveRecord::Base
  has_many :contact_emails
  has_many :contact_calls
  has_many :contact_letters
end

If so, you can then create a last_event method on the Contact model:

def latest_event
  [contact_emails, contact_calls, contact_letters].map do |assoc|
    assoc.first(:order => 'date_sent DESC')
  end.compact.sort_by { |e| e.date_sent }.last
end

Handling nil

When using the latest_event method you will get nil if there are no associated records. There are a couple of ways you can workaround this. The first is to check for nil first with something like:

contact.latest_event && contact.latest_event.date_sent

On late versions of Rails/Ruby you can also use Object#try which will call the method if it exists:

contact.latest_event.try(:date_sent)

I prefer not to use this as it doesn't check for nil but only if the object can respond to a method. This has cause some interesting errors if you expect nil if the object is nil but are calling a method which nil itself responds to.

Finally, my preferred method for the simple case is to use the andand gem which provides Object#andand. This greatly shortens the safe case above and saves calling of latest_event multiple times:

contact.latest_event.andand.date_sent

date_sent, nil and You.

For your example usage of calling to_s(:long), you could either use && or andand:

contact.latest_event.andand.date_sent.andand.to_s(:long)

or

contact.latest_event && contact.latest_event.date_sent.to_s(:long)

The first is safer if date_sent itself may be nil. Without using andand this could be written as:

contact.latest_event &&
  contact.latest_event.date_sent &&
  contact.latest_event.date_sent.to_s(:long)

which is rather complex and unwieldily in my opinion. I would recommend looking into andand

Jason Weathered
this will only look at those contact_emails, contact_calls, and contact_letters that were actually created, right? In which case, there won't be an instance of nil class, right? That's what I get with the other approaches. This is interesting, forgot about .map method...but never fully understood yet either...checking it out!
Angela
So I could then just call Contact.latest_event to get the actual event? So attributes could apply to Contact.latest_event.date_sent?
Angela
this so far works, very nice!
Angela
Hi, well, I have a situation where there may be NO contact_emails, contact_calls, contact_letters, so date_sent applies to a Nil class...how can I correct for that? Thanks!
Angela
Hi Angela. I have added another second to my answer which covers how to handle `nil`.
Jason Weathered
s/second/section/
Jason Weathered