Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Property Injection for non-property setters #224

Closed
ghost opened this issue May 27, 2014 · 8 comments
Closed

Property Injection for non-property setters #224

ghost opened this issue May 27, 2014 · 8 comments

Comments

@ghost
Copy link

ghost commented May 27, 2014

I tried to inject NSPersistentStoreCoordinator(PSC) instance to NSManagedObjectContext(MOC) using TyphoonDefinition#injectProperty:with:.
I guessed it would work well because MOC has 'setPersistentStoreCoordinator' setter method, however, then application crashed with 'TyphoonPropertySetterNotFoundException'.

So, I tried next to inject PSC using injectMethod:parameters:, it works wel;.

Also, I added following codes into TyphoonIntrospectionUtils#setterForPropertyWithName:inClass: below "if (property) {...}", it works well, too:

else if (propertyName.length > 0) {
    /*
     * Construct 'expected' setter name from property name.
     * ex. propertyName => setPropertyName:.
     */
    NSString *firstCharacterInPropertyName = [propertyName substringWithRange:NSMakeRange(0, 1)];
    NSString *restStringInPropertyName = [propertyName substringWithRange:NSMakeRange(1, propertyName.length-1)];
    NSString *expectedSetterName = [NSString stringWithFormat:@"set%@%@:", [firstCharacterInPropertyName uppercaseString], restStringInPropertyName];

    /* Get selector from expected setter selector name, and try to get method, and if exists, return the selector as setter selector.*/
    SEL selector = NSSelectorFromString(expectedSetterName);
    Method method = class_getInstanceMethod(clazz, selector);
    if (method) {
        setterSelector = selector;
    }
}

Its basic idea is that, when class_getProperty returns null, then tries to obtain Method using class_getInstanceMethod with selector with expected setter name with format "set:", and if it return non-null Method, return the selector as setter selector.

(Please overlook poorness of my code. I'm an absolute beginner.)

Is there more suitable way to inject setters which are NOT written with @Property syntax using injectProperty:with?
Or, in these cases, I shouldn't use injectProperty:with: but injectMethod:parameters:?

@jasperblues
Copy link
Member

According to the documentation for this class, there really is no property, but only the getter and setter methods.

Normally this difference would not be noticeable, however there's a difference with regards to the Objective-C run-time. The Objective-C run-time only provides type introspection for properties. Not for method or initializer parameters or return values.

Typhoon has a feature where it can auto-wire properties for you. For example, instead of typing:

[definition injectProperty:@selector(persistentStoreCoordinator) with:something];

. . . we can just type. . .

[definition injectProperty:@selector(persistentStoreCoordinator)];

Similarly, the typhoon autowire macros take advantage of this, for simple "convention over configuration" cases.

. . . but it will only work in the case of a true property.

Therefore, I believe your change could cause this feature to be broken in the above case, as we don't really have a true property.

We _can_ have single parameter methods behave like properties. But, we will need to add some testing and add a make sure that it fails correctly

Example:

Exception: "Property xyz is not a true property, and therefore does not \
have type information. Please use explicit wiring instead of auto-wiring)." 

@jasperblues
Copy link
Member

Would you like to send a pull-request with the change, along with correctly handling the above case? (Default to null-type / raise exception when auto-wiring).

@jasperblues
Copy link
Member

Alternatively we can raise an exception, with clear instructions on how to use method injection instead.

Which is better?

@jasperblues jasperblues added bug and removed bug labels May 27, 2014
@ghost
Copy link
Author

ghost commented May 27, 2014

Thank you.

I'm sorry I haven't understood Auto-Wiring enough.

I will use injectMethod:parameters: instead of injectProperty:with:.

I have no excuse for wasting your time out of my ignorance.

@ghost ghost closed this as completed May 27, 2014
@jasperblues jasperblues reopened this May 27, 2014
@alexgarbarev
Copy link
Contributor

property Injection for non-property setters is good idea, I think I'll do that in future.

@jasperblues
Copy link
Member

Let's leave this open. After discussion with @alexgarbarev we think it would be good to make "properties" that consist of only a getter/setter to behave like ordinary properties. . . instead we'll raise an exception if the user asks for this type to be auto-wired.

I think this will be the most useful, and least surprising behavior.

@jasperblues
Copy link
Member

@masatz we've released this in 2.0.7. Would you mind verifying it for us, so that we can close the ticket?

  • If a class has a 'getter' and corresponding 'setter' method, without formally declaring a property, you can now inject this using normal property-style injections. Its not necessary to use a method injection. However, only explicit wiring will be allowed. Auto-wiring by type will raise an exception.

@jasperblues jasperblues removed the debate label Jun 5, 2014
@ghost
Copy link
Author

ghost commented Jun 5, 2014

I installed Typhoon 2.0.7 and I verified that injectProperty:with works very well with persistentStoreCoordinator on NSManagedObjectContext. Excellent.

Thank you very much for your great work!

@ghost ghost closed this as completed Jun 5, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants