views:

29

answers:

2

I am using a lot of my own validation methods to compare the data from one association to the other. I've noticed that I'm constantly checking that my associations aren't nil before trying to call anything on them, but I am also validating their presence, and so I feel that my nil checks are redundant. Here's an example:

class House < ActiveRecord::Base
  has_one :enterance, :class => Door
  has_one :exit, :class => Door

  validates_presence_of :enterance, :exit

  validate :not_a_fire_hazard
  def not_a_fire_hazard
    if enterance && exit && enterance.location != exit.location
      errors.add_to_base('If there is a fire you will most likely die')
      return false
    end
  end
end

I feel like I am repeating myself by checking the existence of enterance and exit within my own validation.

Is there a more "The Rails Way" to do this?

A: 

I don't know if there is a particular order in which validations are fired. If the validates_presence_of fires first, an easy check might be something like:

# leave enterance and/or exit blank before trying to validate
def not_a_fire_hazard
  if self.errors.count > 0
    raise self.errors.each {|attr, msg| "#{attr} - #{msg}"}.inspect
  end

  # ...
end

if you get a raised exception and your output is a list of errors that contain your validates_presence_of validators then its safe to say that you might not need to check for nil before accessing their members. HOWEVER! I think its a good idea to leave it as is and its not really redundant as much as it is careful programming.

DJTripleThreat
A: 

You also might want to consider using the validates_associated stanza in order to validate whether the associated objects are themselves valid. Also, another cleaner way to go ahead and ensure that both entrance and exit are present (not nil) would be with the following:

validates_presence_of :entrance_or_foo

def entrance_or_foo
    entrance and foo
end

Then you can clean up your fire hazard method to look like the following:

 def not_a_fire_hazard
    if enterance.location != foo.location
      errors.add_to_base('If there is a fire you will most likely die')
    end
 end

You do not need the return false in the definition above.

As noted by Francois in comments, exit is a method defined in the Kernel module. You should rename your class so as to not avoid confusion with the Ruby defined exit method. I've renamed instances of exit to foo in my example code above.

Steve Finkelstein
entrance and exit would quit the server process. exit is a method on Kernel which stops the VM. Ooops
François Beausoleil
+1 Francois, I failed to mention that in my post. I think the idea still stands, though.
Steve Finkelstein
Woops on the exit, this was just an example I made up. I'm not sure how this will fix my problem, as if not_a_fire_hazard is in the validation chain, and entrance or foo is nil, I will still be doing nil.location. Since validation is simultaneous (maybe it won't be in this case?), even if entrance_or_foo doesn't pass, won't not_a_fire_hazard still get called?
lambdabutz