Inject custom keyPath from definition to object?

[alexgarbarev]

I start importing Typhoon to a big project but face with some problems.
Lets start from example:
I have Settings which have property A and property B, and I want to inject these properties to some Service.

Now it looks like:

service = ...
service.a = settings.a;
service.b = settings.b;

I know that I can create class named ServiceSettings with A and B properties and inject it as

service.settings = serviceSettings

But it requires a huge refactoring and breaking some logic.

So my offer is to add ability to inject not whole object from definition, but the part of object (specified by keyPath).
Something like this:

[definition injectProperty:@selector(a) withValueForKeyPath:@"a" forDefinition:[self settings]]

Or is it bad design?

[jasperblues]

I think that sounds quite useful. . . . Another way to achieve this is to make settings a factory component (similar to the themes in the Typhoon sample app:

- (id)settingsA
{
    return [TyphoonDefinition withClass:[Settings class] initialization:^(TyphoonInitializer* initializer)
    {
        initializer.selector = @selector(settingsA);
    } properties:^(TyphoonDefinition* definition)
    {
        definition.factory = [self settingsFactory];
    }];
}


- (id)settingsFactory
{
    return [TyphoonDefinition withClass:[Settings class] properties:^(TyphoonDefinition* definition)
    {
        //etc . . 
    }];
}

. . . you've come up with a short-hand way of doing the example above?

[alexgarbarev]

Thanks for reply.

Yes, now I see that it can be done via current API, it is great.

But for me as for user it is not obvious way. Also as a user I expected that "initialization" block performs before "properties" block, so I not looked for "factory" method in "properties" block.

With keyPath you can do sequence of selectors, like this

[definition injectProperty:@selector(host) withValueForKeyPath:@"availableHostsArray.lastObject" forDefinition:[self settings]]

Yes, it is just short-hand, but imagine how much code needed to implement example below with current API.

[definition injectProperty:@selector(server1Host) withValueForKeyPath:@"server1.host" forDefinition:[self settings]]
[definition injectProperty:@selector(server1Port) withValueForKeyPath:@"server1.port" forDefinition:[self settings]]
[definition injectProperty:@selector(server2Host) withValueForKeyPath:@"server2.host" forDefinition:[self settings]]
[definition injectProperty:@selector(server2Port) withValueForKeyPath:@"server2.port" forDefinition:[self settings]]
[definition injectProperty:@selector(keepAliveTime) withValueForKeyPath:@"keepAliveTime" forDefinition:[self settings]]

I agree that this duplicates current API but I think this way is more obvious for user.

[alexgarbarev]

Let me try to implement it and you'll review it

[jasperblues]

Sounds great. . . perhaps you can use the existing implementation in order to make the short-hand approach.

I was going to suggest that instead of using key-path (Typhoon advertises "no magic strings required") to use a selector, however now I can see from your example that nesting the key-path as you've shown is useful.

Regarding your other feedback about the configuration in the properties block being non-obvious:

• Perhaps its time we had a separate setup/configuration block. Maybe we could do this by moving the property injection methods into their own class, so that a definition is composed of:

1. Top-level configuration
2. Initialization
3. Property configuration

This would also make it easier to use the IDE ctrl-complete to get some self documentation happening, and hopefull be more obvious. . . what do you think?

We'll all be looking forward to seeing what you come up with! Significant contributions means that you get to be featured on the Typhoon website (if you want to).

[alexgarbarev]

Check my pull request.
I implemented two methods to inject part of definition, one IDE-firendly with selector, second one with more flexible keyPath.

- (void)injectProperty:(SEL)selector withDefinition:(TyphoonDefinition*)factoryDefinition selector:(SEL)factorySelector;
- (void)injectProperty:(SEL)selector withDefinition:(TyphoonDefinition*)factoryDefinition keyPath:(NSString *)keyPath;

I tried to deal with current API to implement shorthand, but face with problem of scalar types such a NSInteger or NSRange, so I added new injection type which not requires result class.

Note that my implementation supports injection scalar types, because uses valueForKey and setValue:ForKey: methods. I think that may be better to use this mechanism (KVC) for all properties injections instead of NSInvocation.

About factory property of definition. Which reason to use factory property of definition (except situation from my first message)? Anyway I added shorthand for this:

+ (TyphoonDefinition*)withClass:(Class)clazz factory:(TyphoonDefinition *)definition selector:(SEL)selector;

[jasperblues]

Fantastic work Aleksey!

Can you please add a few test cases and I'll merge in your work right away.

About NSInvocation vs KVC - perhaps you're right. . . originally we started with objc_msgSend, because I wanted raw speed, but that got complicated so we graduated to NSInvocation. . . with our if/else type checking I doubt that we actually get more speed anyway. . . (Objc's heavily peer reviewed/evolved version of KVC is probably as fast as it can be).

If you wanted to simplify that code, we'll accept that too.

[alexgarbarev]

Thanks,

I fixed some bugs and write few tests, now it works correct. But now it looks a bit ugly, because I combined KVC and invocation way.

I absolutely sure that KVC is better. Reasons:

1. KVC does same thing as current NSInvocation solution, so it works with same speed.
2. KVC is not slow. I guess memory allocation for object takes more time. But don't guess - profile!
3. KVC supports keyPath
4. KVC methods can be overridden in object.
5. KVC supports some default structs, not only scalar types. (All structs supported by NSValue)
6. Using KVC makes code more clean and stable (because KVC is old and fully tested).

I'll try to find time to simplify that code using KVC

[jasperblues]

Agreed. . . it could be especially useful to inject things like a CGRect without having to clutter up the API with custom methods.

Let's pull in your changes with test cases, and then one of us (yourself - if you'd like) can convert to KVC.