views:

107

answers:

5

I'm sure this has been asked already, but I can't find the answer.

I have a Project model, which has a belongs_to relationship with my Client model. A client has a name, but a project doesn't necessarily have a client.

In my view, I've got code like this:

<%=h project.client && project.client.name %>

because if the project doesn't have a client then trying to access project.client.name causes a NoMethodError (nil doesn't have a method called name).

The question is, is it acceptable to have this kind of nil checking in the view, or should I be looking for another way around it?

+2  A: 

I think its perfectly acceptable - this is view logic, you are more or less deciding whether or not to show portions of your view, based on whether there is data.

jasonpgignac
+1  A: 

I run into this all the time, and yes it's annoying. Even when there is supposed to never be a nil, dirty data that I inherited sometimes triggers it.

Your solution is one way of handling it. You could also add a method to Project called client_name that displays the client name if it exists, but then you are linking the models together more than some people recommend.

def client_name
  client && client.name
end

You could also make a helper method to do it, but you can end up writing a lot of them. :)

As mentioned by Skilldrick below, this is also useful to add a default string:

def client_name
  client ? client.name : "no client"
end
DGM
This is definitely useful in certain circumstances (e.g. if you want a default name like "no client").
Skilldrick
Another default string implementation without using the ternary operator: `client.name || "no client"`
Eric Hill
One might argue, of course, that since the original question was discussing having a pure mvc model, that by putting a default display string in your model, you may be injecting what's really more view logic into your model, no? That's why I'd have done it the way the questioner did. But, it's all style, this works, too :). I just hate to assume that every view I ever make, I'll want the same default string, you know?
jasonpgignac
+3  A: 

Just use

project.client.try(:name)
Tass
I forgot that one... :) However, it still gets burdensome when you are traversing down 5-6 models deep. :(
DGM
http://en.wikipedia.org/wiki/Law_of_Demeter
Tass
A: 

my hacky solution is to yield a block and rescue the error. Many would say using rescue as logic is very bad form. Just don't use this where you would actually need to know when something is nil and shouldn't be.

In application_helper.rb:

  def none_on_fail
      begin
          return yield
      rescue
          return "(none entered)"
      end
  end

Then in the view:

<%= none_on_fail { project.client.name }  %>

Then methods can be chained as deep as needed and it can be used on any method BUT it will cover up other potential problems with models/relationships/methods if they exist. I would equate it to taking out a splinter with a flamethrower. Very effective with painful consequences if used improperly.

inkdeep
Very Pythonic :)
Skilldrick
A: 

I think these checks can usually be eliminated with a bit of thought. This has the benefit of keeping your view code cleaner, and more importantly, keeping logic out of the view layer, which is a best practice. Some templating engines don't allow any logic in the view.

There are at least a couple of scenarios. Let's say you have a show action that depends on an instance variable. I'd say if the record is not found the controller should not render the html, by redirecting or something else. If you have a loop in the view for an array, use @array.each do |a| end so that it doesn't evaluate if the array is empty. If you truly want an application default in the view, try loading it from a config file, e.g. @page_title || #{@APP_CONFIG['page_title']} (see Railscasts #85). Remember you may want to change these strings later, for example translating the UI.

Those are a couple scenarios where presence checks and usage of try can be avoided. I'd try to avoid them if possible. If you can't avoid them, I'd put the conditional checks in a view helper and add a helper unit test for it to verify (and document) both code paths.

Andy Atkinson