views:

35

answers:

4

I have created a polymorphic association around a model called status.

Some contacts will have a status associated with it. Many won't.

If I try to call a status when one is not there, I get an error. Right now, even if I haven't created a status for the model, it still runs whatever is in the if-end block.

Here's what I am trying, but it's not working:

<% if [email protected]? %>
       <p>Status: <%= @status.find(:last).status %></p>
<% end %>

In the controller, it is defined below:

@status = Contact.find(@contact).statuses

By the way, also open to make code more readable and DRY.

A: 

Making it DRY:

@statuses = @contact.statuses

and

<% unless @statuses.nil? %>
       <p>Status: <%= @status.last.status %></p>
<% end %>

If I understand your code and what you are trying to do correctly, wouldn't this help you?

<% if @statuses.length > 0 %>
       <p>Status: <%= @status.last.status %></p>
<% end %>
Jakub Hampl
A: 

One option is to not have to check for the existence of a status, by giving all contacts (when a contact is created) a default status that does nothing.

You could call this the Null Object idiom: a object that you use where you'd otherwise use null (C, C++, Java), or NULL (SQL) -- this is in fact what Smalltalk does with nil; nil is a special object, an instance of the class UndefinedObject.

The advantage to this idiom is that you don't have to check for, and have special handling for, the "object does not exist" condition. This makes for cleaner code, and is more object oriented as well, because your Null Object instance, not the calling code, can determine what to do when its methods are called.

Here's a Java example, using the Chain of Responsibility Pattern. Chain of Responsibility basically means having a list of handlers, each of which handle something (a command) or pass it to the next handler in the chain. We'll make a Null Object handler that just does nothing when it's asked to handle something.

First, the general contract:

interface Handler {
  void handle( Command c ) ;
}

Then the Null Handler, with a (horrors!) singleton:

class NullHandler implements Handler {
  public static void NullHandler singleton = new NullHandler();

  void handle( Command c ) { /*no-op*/}
}

Then the base class for every other kind of Handler (this also employs the template Method pattern, bust that's just an implementation detail):

abstract class BaseHandler implements Handler {
  private Handler next;

  public BaseHandler( Handler next ) {
    this.next = next ;
  }

  public BaseHandler() {
    this( NullHandler.singleton ) ;
  }

  public void handle( Command c ) { 
    if( canHandle( c ) {
       doHandle( c ) ;
    } else {
       next.handle( c ) ;
    }
  }


  //Template Method hooks
  abstract boolean canHandle( Command c ) ;
  abstract void doHandle( Command c ) ;
}

So what we get here is limited -- we can do next.handle( c ) ; rather than if next != null next.handle( c ) ;. But as we add more code, the value becomes more apparent.

Let's say we want to print our Chain of Responsibility. One way to do this would be to externalize iterating the Chain in our code, testing each time to see if next == null.

But better, more object orientedly, we can let the Chain do it itself. Again, in BaseHandler's print, we'd print that instance's name and then call next's print method. The print method in NullHandler would print nothing, or perhaps, "end of chain".

tpdi
A: 

I actually came up with this:

<% unless @status.empty? %>
       <p>Status: <%= @status.find(:last).status %> <%= @status.find(:last).created_at.to_s(:long) %></p>
<% end %>   
Angela
A: 

The problem on the opening parent that Contact.statuses returns an array - allways. If a contact has no statuses, the array is empty ([]) - but it is not nil.

I see that you are using empty? now.

I would suggest making it even more DRY, by moving things to the model. For example, take this simple method:

class Contact < ActiveRecord::Base
  ...
  def last_status
    @last_status ||= statuses.last
  end
end

With this you won't have to calculate the @statuses variable on the controller. It will also cache statuses.last on the @last_status variable, so statuses.last is called only once. Remove the @last_status ||= part if you don't wish this to happen.

Then your view can be done more or less as you initially intended:

<% unless @contact.last_status.nil? %>
  <p>Status: <%= @contact.last_status.status %> <%= @contact.last_status.created_at.to_s(:long) %></p>
<% end %>   
egarcia