atomicobject / objection

A lightweight dependency injection framework for Objective-C
http://www.objection-framework.org
MIT License
1.85k stars 223 forks source link

Ability to specify a class method as an objection_initializer #61

Closed zats closed 10 years ago

zats commented 10 years ago

I implemented ability to have a class method as objection_initializer. Also this pull request contains couple of changes that I would love to hear someone's opinions on.

  1. I added an explicit __autoreleasing ARC qualifier for the returning instance since ARC was getting sometimes.
  2. objection_initializer's method result is being stored in the returning instance variable. This one was also aimed to fix an edge-case when initialization method wouldn't return the same instance as alloc (which is obviously is not a recommended practice). Also it address all cases when a static factory method is used.

Please let me know if approach I used seems valid to you.

Thank you.

jdewind commented 10 years ago

Good idea.

I'll merge it soon and add tests.

NachoSoto commented 10 years ago

I'm sorry to inform you that this seems to have broken Objection :/

NSMethodSignature *signature = [klass methodSignatureForSelector:initializer];

When using @selector(init) this returns - [NSObject init], as an instance method, but because of how this was implemented it's calling that method on the class instead of the instance.

zats commented 10 years ago

@dewind I just noticed that __autoreleasing qualifier didn't make it through the pull request. Please see the Mantle's code for a good explanation why __autoreleasing is needed in this case. I might be mistaken though, but it seems like a very similar case to me.

jdewind commented 10 years ago

The reason I didn't think it was applicable is that I didn't see the instance being passed via (id *). However, I I just noticed that it does in-fact do that in getReturnValue

For reference (ARC docs):

__autoreleasing to denote arguments that are passed by reference (id *) and are autoreleased

jdewind commented 10 years ago

Fixed in commit 455489f67e36847baca35e25a4524b56ac14d70a