Rails NoMethodError in loop when method exists

Good day all.

I'm running into a bit of a problem getting a script running on my production environment, even though it works just fine on my dev box. I've verified that all the requisite gems and such are the same 开发者_开发知识库version.

I should mention that the script is intended to be run with the script/runner command.

Here is a super-condensed version of what I'm trying to do, centered around the part that's broken:

def currentDeal
 marketTime = self.convertToTimeZone(Time.new)
 deal = Deal.find(:first, :conditions => ["start_time  ? AND market_id = ? AND published = ?", marketTime, marketTime, self.id, 1])
 return deal
end

markets = Market.find(all)
markets.each do |market|
  deal = market.currentDeal
  puts deal.subject
end

Now convertToTimeZone is a method attached to the model. So, this code works just fine on my dev machine, as stated. However, attempting to run it on my production machine results in:

undefined method `subject' for nil:NilClass (NoMethodError)

If, however, I go into the console on the production box and do this:

def currentDeal
  marketTime = self.convertToTimeZone(Time.new)
  deal = Deal.find(:first, :conditions => ["start_time  ? AND market_id = ? AND published = ?", marketTime, marketTime, self.id, 1])
  return deal
end

market = Market.find(1)
deal = market.currentDeal
puts deal.subject

It returns the correct value, no problem. So what is going on?

This is on rails v 2.3.5, on both machines.

Thanks for any help

---

You are looping though all Markets on your production code, but your test snippet is only looking for one. The problem is that one of your Markets in your database has a currentDeal of nil (it has no object associated with it).

Run this on your production console instead.

markets = Market.find(all)
markets.each do |market|
  deal = market.currentDeal
  if deal
    puts deal.subject
  else
    puts "NO currentDeal for Market with id: #{market.id}"
  end
end

This will tell you exactly which Market record is exploding without a currentDeal.

---

So the question is how to fix it? Either all Markets are expected to have a currentDeal, or sometimes they don't and that's ok. If Market's should always have a currentDeal, then you need to adjust your validations to now allow a Market to be saved without a currentDeal. But given that the currentDeal is a time based thing, I would be that there is times when no deal is scheduled and therefore currentDeal will return nil.

So, more likely, you need to allow for the current deal to be nil. Your test code doesn't do this. It asks the market for the deal, and then the deal for it's subject. If the market return a nil deal, then you immediately ask nil for it's subject and you get the exception because nil does not have a method named subject. A few simple ways to nil protect you code:

deal = market.currentDeal

# simple if
if deal
  puts deal.subject
end

# rails try method returns nil if the receiver is nil
# or executes the method if the object supports it
puts deal.try(:subject)

# ternary
puts deal ? deal.subject : "NO DEAL!"

# conditional execution
puts deal && deal.subject

---

Lastly, a ruby tip. This method is more complicated than it needs to be.

def currentDeal
  marketTime = self.convertToTimeZone(Time.new)
  deal = Deal.find(:first, :conditions => ["start_time  ? AND market_id = ? AND published = ?", marketTime, marketTime, self.id, 1])
  return deal
end

Ruby will always return the last expression's result in a method, and a has based conditions finder will clean up that query quite a bit.

def currentDeal
  marketTime = self.convertToTimeZone(Time.new)
  Deal.find(:first, :conditions => ["start_time > ? AND market_id = ? AND published = ?", marketTime, marketTime, id, true])
end

But this looks more like an association anyway. So you may want to use the association methods to clean this up further.

---

Clearly you are calling nil.subject, so Deal.find is returning nil in the production code. Your test case is only looking at one specific Market object, but the general case loops through Market objects. Your code needs to handle not finding a currentDeal for a Market object