sxross / MotionModel

Simple Model and Validation Mixins for RubyMotion
MIT License
192 stars 67 forks source link

`_get_attr:': undefined method `name' for nil:NilClass (NoMethodError) #40

Closed gpeal closed 11 years ago

gpeal commented 11 years ago

I'm getting '_get_attr:': undefined methodname' for nil:NilClass (NoMethodError)` after upgrading to 0.4.4 and RubyMotion 2.0. The code is here https://github.com/sxross/MotionModel/blob/master/lib/motion_model/model/model.rb#L624

The closest offending code in my app is

def coordinate
    CLLocationCoordinate2D.new(latitude, longitude) # this line
end
sxross commented 11 years ago

That line of code doesn't look (from here) like it would even interact with MotionModel. Have you run it in the debugger and done a backtrace to see what's going on?

gpeal commented 11 years ago

This is the full trace

2013-05-12 11:12:20.700 Brew Mate[16318:c07] *** Terminating app due to uncaught exception 'NoMethodError', reason: 'model.rb:624:in `_get_attr:': undefined method `name' for nil:NilClass (NoMethodError)
    from model.rb:313:in `block in define_accessor_methods:'
    from brewery.rb:23:in `coordinate'
    from local_view_controller.rb:106:in `block in search:'
    from local_view_controller.rb:106:in `block in search:'
    from query.rb:358:in `call_delegator_with_response'
    from query.rb:128:in `connectionDidFinishLoading:'
'
sxross commented 11 years ago

Can you look at your version of MotionModel (in your installed gems), and make sure it reads:

    define_method(name.to_sym)        { _get_attr(name) } unless allocate.respond_to?(name)

There was a new problem introduced in RM2.0 that Watson fixed in MotionModel and if this uses alloc instead of allocate, I need to rebuild the gem.

Thanks

On May 12, 2013, at 9:14 AM, Gabriel Peal notifications@github.com wrote:

from model.rb:313:in `block in define_accessor_methods:'
gpeal commented 11 years ago

Yes, I have version 0.4.4 My line model.rb:313 reads: define_method(name.to_sym) { _get_attr(name) } unless allocate.respond_to?(name)

sxross commented 11 years ago

Is there any way you can reduce this to a small repro case or something that can be shared? I'd like to see it on context. It's exactly the same issue Watson addressed in his last pull request and I think there may be more to the problem.

Thanks

Sent from my iPhone

On May 12, 2013, at 10:42 AM, Gabriel Peal notifications@github.com wrote:

Yes, I have version 0.4.4 My line model.rb:313 reads: define_method(name.to_sym) { _get_attr(name) } unless allocate.respond_to?(name)

— Reply to this email directly or view it on GitHub.

gpeal commented 11 years ago

Here https://github.com/gpeal/MotionModelTest

sxross commented 11 years ago

I raised ticked #729 against RM because this is a behavior change from 1.x. You can check it out on the tracker if you want to add information. I take it this is a showstopper for you. I think you can mark it as such.

On May 12, 2013, at 1:05 PM, Gabriel Peal notifications@github.com wrote:

Here https://github.com/gpeal/MotionModelTest

— Reply to this email directly or view it on GitHub.

gpeal commented 11 years ago

Can you link to the ticket? Not seeing it here: http://hipbyte.myjetbrains.com/youtrack/issues/RM

stevebatcup commented 11 years ago

I get the same error when using the BubbleWrap::KVO observe method.

sxross commented 11 years ago

This all used to work under 1.x, right? I'd like HipByte to sort out why code that was working isn't now. — Sent from Mailbox for iPhone

On Mon, May 13, 2013 at 8:54 AM, willwork4food notifications@github.com wrote:

I get the same error when using the BubbleWrap::KVO observe method.

Reply to this email directly or view it on GitHub: https://github.com/sxross/MotionModel/issues/40#issuecomment-17821047

DougPuchalski commented 11 years ago

What's in _column_hashes?

gpeal commented 11 years ago

Yes, this was working prior to 2.0

gpeal commented 11 years ago

Is there an update on this?

sxross commented 11 years ago

I feel it's a RM issue and submitted a ticket. It's 100% repro but I haven't heard anything back.

Sent from my iPad

On May 20, 2013, at 3:20 PM, Gabriel Peal notifications@github.com wrote:

Is there an update on this?

— Reply to this email directly or view it on GitHub.

gpeal commented 11 years ago

Can you link to the RM ticket?

sxross commented 11 years ago

Sorry: http://hipbyte.myjetbrains.com/youtrack/issue/RM-137

ys commented 11 years ago

Everything is fine if I turn the @_column_hashes to @@_column_hashes But I suppose I'm then defining this in the wrong place...

DougPuchalski commented 11 years ago

That would bust inheritance.

On Thursday, May 23, 2013 at 3:06 PM, Yannick Schutz wrote:

Everything is fine if I turn the @_column_hashes to @@_column_hashes But I suppose I'm then defining this in the wrong place...

— Reply to this email directly or view it on GitHub (https://github.com/sxross/MotionModel/issues/40#issuecomment-18374964).

ys commented 11 years ago

I know… The problem comes from the instance variable that seems to be nil when a NSKVONotifying proxy subclasses the model… In my case at least.

On 24 May 2013, at 01:01, Doug Puchalski notifications@github.com wrote:

That would bust inheritance.

On Thursday, May 23, 2013 at 3:06 PM, Yannick Schutz wrote:

Everything is fine if I turn the @_column_hashes to @@_column_hashes But I suppose I'm then defining this in the wrong place...

— Reply to this email directly or view it on GitHub (https://github.com/sxross/MotionModel/issues/40#issuecomment-18374964).

— Reply to this email directly or view it on GitHub.

sxross commented 11 years ago

Ahhhhh @ys. So you are using KVO on a MotionModel class? If so, is there any chance you can observe the MotionModel change notifications instead?

ys commented 11 years ago

I'll give a look this week end.

On 24 May 2013, at 19:12, s.ross notifications@github.com wrote:

Ahhhhh @ys. So you are using KVO on a MotionModel class? If so, is there any chance you can observe the MotionModel change notifications instead?

— Reply to this email directly or view it on GitHub.

gpeal commented 11 years ago

@ys any luck? This is a show stopping bug for my app.

ys commented 11 years ago

I didn't had time to try… I had to switch to a simpler solution because of that. But will try to do a project with motionmodel asap. Hope someone can help about this blocking bug

On 28 May 2013, at 20:29, Gabriel Peal notifications@github.com wrote:

@ys any luck? This is a show stopping bug for my app.

— Reply to this email directly or view it on GitHub.

sxross commented 11 years ago

@gpeal -- are you using KVO as well?

gpeal commented 11 years ago

@sxross I'm not using it explicitly. I can't guarantee that MKMapViewAnnotation or MotionModel isn't internally somehow though.

sxross commented 11 years ago

@gpeal This code works in your example MotionModelTest project:

class MapViewController < UIViewController
  def viewDidLoad
    self.view = MKMapView.alloc.init
    brewery = Brewery.create(id: 'ABCD',
                             name: 'Test Brewery',
                             latitude: 41.88,
                             longitude: -87.65)
    @brewery = WeakRef.new(brewery)
    self.view.addAnnotation(@brewery)
  end

end

Does this help?

gpeal commented 11 years ago

It appears the brewery model is using KVO. Inspecting the annotation passed to mapView:viewForAnnotation, it appears as "Brewery: #<NSKVONotifying_WeakRef:0x1acdf740>" (The WeakRef is new in response to @sxross's comment). Apple's docs indicate that MKAnnotationViews automatically use KVO: http://developer.apple.com/library/ios/#documentation/userexperience/conceptual/LocationAwarenessPG/AnnotatingMaps/AnnotatingMaps.html

Making it WeakRef now gives me this: NSGenericException: <WeakRef: 0x1acdf740> must implement title when canShowCallout is YES on correspoding view The model does implement the required title and coordinate methods

sxross commented 11 years ago

I don't know much about the map kit but it would seem if you implement a title method that returns some string you'd be fine. But... that said, I wonder how KVO is creeping in? Not that KVO is a bad thing, but MotionModel has to be proxied with care.

Try implementing a title method and see where it gets you.

gpeal commented 11 years ago

@sxross the title method is implemented. Also, look at the Important box just below Listing 5-2 in the apple docs link I posted. That may be where KVO is coming from.

sxross commented 11 years ago

Duly noted. I can reliably build your example using weakref so there must be something else in your project I can't see. It would be helpful if you could create a "shadow" model not using MotionModel, initialize it by copy from your MorionModel object and pass that to MapKit. If that works, then count me puzzled.

gpeal commented 11 years ago

@sxross I was able to make a workout by doing 2 things Making a dummy annotation class that looks like this

class DummyAnnotation
  attr_accessor :title, :latitude, :longitude, :annotation_url

  def coordinate
    CLLocationCoordinate2D.new(latitude, longitude)
  end
end

and using a WeakRef to that when adding the annotation

sxross commented 11 years ago

Hey! I really appreciate this. It's the "shadow" class I was talking about. The question still lurks: Why doesn't this work as you originally implemented it, so I'll keep the issue open.

On May 30, 2013, at 12:32 PM, Gabriel Peal notifications@github.com wrote:

@sxross I was able to make a workout by doing 2 things Making a dummy annotation class that looks like this

class DummyAnnotation attr_accessor :title, :latitude, :longitude, :annotation_url

def coordinate CLLocationCoordinate2D.new(latitude, longitude) end end and using a WeakRef to that when adding the annotation

— Reply to this email directly or view it on GitHub.

stabenfeldt commented 11 years ago

@gpeal, I cloned your MotionModelTest repo. But it fails on reason: 'model.rb:624:in_get_attr:': undefined method name' for nil:NilClass Do you have a working version of it somewhere? I have exactly the same problem in my RubyMotionBeerSample project. :)

stabenfeldt commented 11 years ago

Not sure if it gives any value to what you guys already know, but I get the same error on my project.:

2013-06-03 08:02:32.919 Scaffold[6676:c07] model.rb:625:in `_get_attr:': undefined method `name' for nil:NilClass (NoMethodError)
    from model.rb:313:in `block in define_accessor_methods:'
    from beer.rb:18:in `coordinate'
    from second_view_controller.rb:21:in `block in viewDidLoad'
    from second_view_controller.rb:21:in `viewDidLoad'

beer.rb

15  def coordinate
16    loc           = CLLocationCoordinate2D.new
17    loc.latitude  = location[:latitude]
18    loc.longitude = location[:longitude]
19    loc
20  end

Testing in the console:

(main)> Beer.all.each  { |b|  p b.coordinate }
#<CLLocationCoordinate2D latitude=59.9234619140625 longitude=10.75830078125>
#<CLLocationCoordinate2D latitude=59.9234619140625 longitude=10.75830078125>
#<CLLocationCoordinate2D latitude=59.9234619140625 longitude=10.75830078125>
=> [Beer#1:0xa34a130, Beer#2:0xa5a5e60, Beer#3:0xa5a90b0]
stabenfeldt commented 11 years ago

Using WeakRef:

2013-06-03 21:32:52.927 Scaffold[23012:c07] *** 
Terminating app due to uncaught exception 'RuntimeError', reason:
 'second_view_controller.rb:23:in `block in viewDidLoad': NSGenericException: 
<WeakRef: 0xffd2470> must implement title when canShowCallout is YES on correspoding view 
<MKPinAnnotationView: 0xff48a20; frame = (-8 -34; 32 39); layer = <MKLayer: 0xff48830>> visible:0 +59.92346191, +10.75830078 (RuntimeError)
    from second_view_controller.rb:21:in `viewDidLoad'

title is implemented:

class Beer
  include MotionModel::Model
  include MotionModel::ArrayModelAdapter

  attr_accessor :title

  columns location: :hash,
          name:     :string,
          url:      :string, 

  def title;      name;       end
  def url;        url;        end
gpeal commented 11 years ago

@stabenfeldt my test project was created to reproduce the error. I had the exact same problem as you including the canShowCallout one. Glad it wasn't just me.

I was able to work around it. Just look at my last comment from 4 days ago. It is definitely a problem though.

sxross commented 11 years ago

Your workaround is good. The problem has to do with proxying (and that happens through the magic of your passing the model off to MapKit) and the ability of the MotionModel internals to resolve dynamic accessors. That's why I believe the simple class works and the MotionModel one doesn't -- because simple classes actually have these instance variables.

As before, I'm going to leave this open because it's likely to be a broader issue.

stabenfeldt commented 11 years ago

@gpeal, thanks for the tips! I tried that, but I still get the must implement title when canShowCallout is YES on correspoding viewerror from map_controller.rb:21:in `viewDidLoad'

I would be very grateful if you could take a quick look and perhaps spot what I´m doing wrong: DummyAnnotation.rb beer.rb map_controller

gpeal commented 11 years ago

Add that DummyAnnotation class I posted above. Then add attr_accessor :dummy_annotation to beer.rb, Then, instead of adding setting @brewery as the annotation, copy over the annotation fields to it's dummy_annotation and set the annotation as WeakRef.new(@brewery.dummy_annotation)

stabenfeldt commented 11 years ago

Cool, it worked! :+1:

Had to skip WeakRef, though, as I kept on getting the missing title error when using it:

    Beer.open_now.each do |beer| 
      shadow = DummyAnnotation.new
      shadow.title     = beer.title
      shadow.longitude = beer.location[:longitude]
      shadow.latitude  = beer.location[:latitude]
      beer.dummy_annotation = shadow
      #@brewery = WeakRef.new(beer.dummy_annotation)   # This does not work
      self.view.addAnnotation(beer.dummy_annotation)   # This works!
    end
sxross commented 11 years ago

This is an annoying problem and I'm still chasing it -- I don't want you to think I'm ignoring it. Here's what I've observed: Even though there are only 3 breweries being added, MapKit is trying to get the name and coordinate for a 4th. But, of course, the 4th doesn't exist and is therefore nil.

I'm not sure why the WeakRef solution works for me and not for you, but it does. Even so, it's important to understand why a plain class works differently from our models.

sxross commented 11 years ago

MotionModel 0.4.2 works as you would expect. There was something introduced between 0.4.2 and now that causes this issue. If you want to streamline your code, revert to version 0.4.2, bundle, and you're good to go.

stabenfeldt commented 11 years ago

Nice, thanks for really digging into this issue! :-)

Martin Stabenfeldt On 4 Jun 2013 23:39, "s.ross" notifications@github.com wrote:

MotionModel 0.4.2 works as you would expect. There was something introduced between 0.4.2 and now that causes this issue. If you want to streamline your code, revert to version 0.4.2, bundle, and you're good to go.

— Reply to this email directly or view it on GitHubhttps://github.com/sxross/MotionModel/issues/40#issuecomment-18940826 .

DougPuchalski commented 11 years ago

I finally had a look at this again. The problem is indeed inheritance. A subclass of a model does not inherit the columns of its parent, so the anonymous subclass (i.e. NSKVONotifying_RBAnonymousXX) will be un-configured.

I'm not sure how (if?) Rails does this, perhaps an inherited hook?

sxross commented 11 years ago

Closing. Migrated to issue #53