3

controller

def show
  @dailies = Daily.where(store_id: params[:store]).order(created_at: :asc).by_month_year(params[:month],params[:year])
  @services = Service.all.order(id: :asc)
end

view

<%= daily_data_for(params[:store], params[:month], date.day, params[:year], 'starting_safe') %>

helper

def daily_data_for(store, month, day, year, field)
  Daily.where(store_id: store, month: month, day: day, year: year).first().send(field)
end

I'm getting the error undefined method 'starting_safe' for nil:NilClass when trying to display ActiveRecord object attribute in the view. I'm a but confused as it shouldn't be nil, because if I remove .send(field) from the helper, I get #<Daily:0x00007fea597aad50>. So it's returning an object. So then I inspect the Activerecord Object in the view:

daily_data_for(params[:store], params[:month], date.day, params[:year], 'starting_safe').inspect

Which displays #<Daily id: 1030, store_id: 1, created_at: "2018-10-01 18:20:07", updated_at: "2018-10-01 18:20:07", starting_safe: 0.1e1, ending_safe: 0.1e1, total_in: 0.1e1, total_out: 0.1e1, sum: 0.1e1, starting_drawer: 0.1e1, ending_drawer: 0.1e1, over_short: 0.1e1, year: "2018", month: "10", day: "1">

It's getting the right record and returning it, but then I get undefined method of nil class when I try to access starting_safe.

Why am I unable to display starting_safe in the view? I'd like to be using the instance variable @dailies in the helper instead of querying the database again, but I get the same errors. I've tried in the console and everything works fine. I'm a bit lost as to what's going on here.

halfer
  • 19,824
  • 17
  • 99
  • 186
David S
  • 325
  • 4
  • 15

2 Answers2

2

I'm pretty sure that the record you inspect (Daily with id=1030) and the failing [non-existing] record are two different records. Here's how you can find out. Amend your method like this, temporarily:

def daily_data_for(store, month, day, year, field)
  daily = Daily.where(store_id: store, month: month, day: day, year: year).first
  raise "Daily not found for #{store.inspect}, #{month.inspect}, #{day.inspect}, #{year.inspect}" unless daily
  daily.send(field)
end

Now the error message should be much more revealing.

Sergio Tulentsev
  • 226,338
  • 43
  • 373
  • 367
  • It's returning `0.1e1` which is the value of the starting_safe, and all of the other attributes are also now being displayed. Can you explain why that worked?? Whoops, i had .inspect in the view still. It properly displays `1.0` now. – David S Oct 31 '18 at 19:43
  • @DavidS Did it work, though? it simply ignored the error. Have you checked the logs to see which parameters were logged? Also check out my second variant. The error will be in your face, won't miss it :) – Sergio Tulentsev Oct 31 '18 at 19:46
  • 1
    @DavidS This is currently not really a fix. More something to find your missing record. It should be printed in the server log. The *NoMethodError* exception is still raised, but caught printing out the input values that caused the exception and returning `nil`. If you'd like to return `nil` if no object is found save it into a variable and call *send* only if it contains an object. `daily = Daily.find_by(store_id: store, month: month, day: day, year, year) or return`. – 3limin4t0r Oct 31 '18 at 19:49
  • Adding the rescue line 'fixes' it. I'll have to dig around more and see if I can figure out what exactly happened. I'm still a bit new to rails/ruby, so I'm still learning how to properly debug. – David S Oct 31 '18 at 19:50
  • 1
    @JohanWentholt: removed that rescue/log snippet in favor of the more foolproof one – Sergio Tulentsev Oct 31 '18 at 19:51
  • @DavidS: it wasn't meant to be a fix. It's a debugging measure. Don't leave it there. Try the current snippet – Sergio Tulentsev Oct 31 '18 at 19:54
  • @SergioTulentsev It was fine, but should `raise` without arguments inside the rescue block to re-raise the current exception after printing your custom message. – 3limin4t0r Oct 31 '18 at 19:56
  • `find_by` is a bit cleaner in my opinion so that one does not miss the `first` at the end. @JohanWentholt `or return` is dirty to me; given the rails version `find_by(...)&.send(field)` is less of an antipattern – engineersmnky Oct 31 '18 at 19:56
  • 2
    @DavidS it is just a guess that as you probably loop thru each day of the month, for some day you have zero records, hence first() returns nil and then the error is raised. – Mihail Panayotov Oct 31 '18 at 19:58
  • 3
    @engineersmnky `send` is dirty too. For active records there's cleaner `read_attribute(field)` :) – Sergio Tulentsev Oct 31 '18 at 20:01
  • @engineersmnky I agree with you that using the safe navigator is better here, since the only thing to do after finding the record is sending the method name. However I don't consider `or return` dirty. `or` and `and` are fine for use in flow control. As long as you don't use them in boolean context. https://github.com/rubocop-hq/ruby-style-guide/pull/730 – 3limin4t0r Oct 31 '18 at 20:02
  • @SergioTulentsev very nice (I was unaware of that method) as long as it is a true `attribute` which in this case it definitely appears to be – engineersmnky Oct 31 '18 at 20:03
  • @MihailPanayotov I believe that was the issue. Thank you everyone that was involved. You've all contributed valuable tips! – David S Oct 31 '18 at 20:06
  • @johanwentholt I have a general distaste for early returns (excepting guard clauses) but that's just my opinion :) – engineersmnky Oct 31 '18 at 21:53
0

The error undefined method 'starting_safe' for nil:NilClass is specifically telling you that you can't call starting_safe on nil. Since your helper method calls a scope .where method, calling .first on that doesn't insure that it won't be an empty activerecord relation object. If it is empty, then calling .first on it will return nil. So you might wanna just use safe navigation to be safe.

def daily_data_for(store, month, day, year, field)
  Daily.where(store_id: store, month: month, day: day, year: year).first&.send(field)
end

Or if you happen to be on ruby version < 2.3 just use a .try

Daily.where(store_id: store, month: month, day: day, year: year).first.try(:send, field)
#try will only run if first is not nil
lacostenycoder
  • 10,623
  • 4
  • 31
  • 48
  • 1
    Good tips, but, of course, if they expect/allow records to be missing. Sometimes missing record is a legitimate failure state (and so the app should crash loudly) – Sergio Tulentsev Oct 31 '18 at 20:12
  • @SergioTulentsev yes agreed. However, the OP asked WHY they were getting the error. I suppose I didn't need to explain how they might fix it. However, IMHO the method's use in the view doesn't make much sense here. – lacostenycoder Oct 31 '18 at 20:15
  • 1
    The method call in the view looks fine to me. Not any more complex than, say, `options_for_select`. The N+1 is the real problem here. – Sergio Tulentsev Oct 31 '18 at 20:17