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

Allow parameterized factory methods #47

Closed
rhgills opened this issue Aug 3, 2013 · 16 comments
Closed

Allow parameterized factory methods #47

rhgills opened this issue Aug 3, 2013 · 16 comments

Comments

@rhgills
Copy link
Contributor

rhgills commented Aug 3, 2013

See:

experimental branch: https://github.com/jasperblues/Typhoon/tree/parameterized-provider

related issue: #42

@rhgills
Copy link
Contributor Author

rhgills commented Aug 3, 2013

@jasperblues
Copy link
Member

That's perfect! Well done. This also relates to #23 - Autofac Style parameter injection.

I think we'll have to move Typhoon to its own GH account - its becoming your project as much as mine :) +1

@rhgills
Copy link
Contributor Author

rhgills commented Aug 5, 2013

Thanks! Indeed it does. I recently added the ability of objects to implement <TyphoonInjectionAware> and have the TyphoonAssembly (well, the BlockComponentFactory) that created them passed as a parameter to setAssembly:, which lets you use the assembly as a provider, albeit one that is extremely general and can provide any type known to the assembly. Or you can make your own factory objects, which themselves can be TyphoonInjectionAware, just for a narrower interface and type safety, and call through to the underlying assembly in those classes only. No more need to call the defaultAssembly singleton and this makes the dependency for those objects that would otherwise use the defaultAssembly as a factory more obvious.

You're too kind! You started an inspiring project, I'll give you that :)

@jasperblues
Copy link
Member

I considered from day 1, but was hesitant because I wanted Typhoon to be "non-invasive". . . in retrospect I agree now that its better though.

I often use Typhoon like a storyboard to transition from one VC to the next. . this feature makes perfect sense in those kinds of cases - eg where you don't want to inject the subsequent VC as a dependency in the previous. . (Even if circular dependencies was bullet-proof).

@jasperblues
Copy link
Member

Are you on Skype or gtalk?

@rhgills
Copy link
Contributor Author

rhgills commented Aug 7, 2013

Neither, unfortunately.

@jasperblues
Copy link
Member

I made a start on this, but then got stuck here: http://stackoverflow.com/questions/18295758/imp-with-unknown-number-of-parameters

Tried using NSProxy instead of swizzling, and IMP_ImplementationWIthBlock. . but ran into issues there. . so it seems we'd be getting into libffi territory with feature.

@rhgills
Copy link
Contributor Author

rhgills commented Aug 29, 2013

Looking over the parameterized-provider branch, it seems the route I took was using an implementation function with variable arguments. As mentioned in that SO thread, this is probably a horrible hack. But it seems to work, so long as only objects are passed as parameters. This isn't terrible, as unboxing could be performed within the definition method on the assembly.

class_addMethod(self, sel, (IMP)myMethodIMP, "@@:?");

id myMethodIMP(id self, SEL _cmd, ...)
{
    NSMutableArray *array = [NSMutableArray arrayWithCapacity:0];
    int numArgs = [[TyphoonBlockComponentFactory class] numberOfUserArgumentsInSelector:_cmd];

    va_list args;
    va_start(args, _cmd);
    id anArg = nil;
    for (int i = 0; i < numArgs; i++) {
        anArg = va_arg(args, id);
        [array addObject:anArg];
    }
    va_end(args);

    return [self componentForKey:NSStringFromSelector(_cmd) arguments:array];
}

I'm not familar with libffi (beyond the first paragraph of their home page).

@jasperblues
Copy link
Member

Oh nice - this is what I was originally looking for, but couldn't find an example of an IMP with va_args.

After that I tried using fowardInvocation, but realized it was tricky to proxy a class from within itself. . . then I got on to libffi. . . I haven't learned how to use libffi yet . .. my only experience with it was in checking out the aspects in libextobjc .. . these were not yet stable.

I think it would be possible to inspect the argument types to detect primitives. And either throw an exception or box transparently. . .

@rhgills
Copy link
Contributor Author

rhgills commented Aug 31, 2013

Conceptually I like the idea of forwardInvocation much more - we're actually dynamically handling things, instead of adding a new instance method to the class whenever one is called that doesn't exist. But I haven't tried it yet so I have no appreciation for the complexities. If it's hard to proxy the BlockComponentFactory from within itself, perhaps we could add another class? Say a BlockComponentFactoryProxy whose only job is to translate the invocations made on it into componentForKey:arguments: calls on the BlockComponentFactory.

@jasperblues
Copy link
Member

There's two places where proxy could work:

In TyphoonAssembly - where we dynamically allocate a key and register with the container. . . . This is the one I had trouble with, because sub-classes call [initializer injectWithDefinition:[self someComponent]]; . . . . self here ends up being the wrong self. . . didn't go through the proxy machinery :(
In TyphoonComponentFactory where we allow using the interface to resolve keys. . . I already changed this one to use forwardInvocation. . . has been working ok.

@rhgills
Copy link
Contributor Author

rhgills commented Aug 31, 2013

Right, just noticed the change you made to the BlockComponentFactory today when I had a chance to catch up on all the commits I had missed. That's what I was thinking, just inlined into the current class. Might get a bit more complicated with the parameterized provider, but still simple compared to the Assembly, which I wasn't thinking about.

Currently we have to swizzle the Assembly methods so that the subclasses, when they call [initializer injectWithDefinition:[self someComponent]], will actually go through the proxy machinery instead of calling the real method -(id)someComponent defined on the subclass. So in this case it wouldn't go through forwardInvocation. Got it!

@rhgills
Copy link
Contributor Author

rhgills commented Oct 30, 2013

Lately I've been thinking that a better solution to the duplication problem is Spring-like parent definitions. You miss out on the possibility of providing a dependency/definition at runtime when calling the assembly, but it seems like it would be simpler to implement, especially for supporting XML as well as Assembly definitions.

See #90.

@jasperblues
Copy link
Member

This abstract/inherited definitions would certainly be very easy to implement compared to the parameter injection. And very useful. However, do they address all the same problems that the auto-fac style parameter injection addresses? I didn't think so.

In any case - you're right though - we should be going for the low hanging fruit first.

@rhgills
Copy link
Contributor Author

rhgills commented Oct 31, 2013

(a reply to #89 (comment), but trying to keep it in the correct thread)

The benefit is neatly summarized in the subtitle of that documentation page: "Let components create instances of other components without hand-coding factories."

We can do that now with Typhoon, subject to the additional restriction that ALL constructor arguments must be supplying by Typhoon (and thus should be true depnedencies, not dynamic run-time configuration).

It would be nice, certainly, but as a workaround:

Make any dynamic run-time configuration a property, not a constructor argument, and set it manually after retrieving the constructed instance of Typhoon.

@jasperblues
Copy link
Member

Good analysis. Given that:

#90 moves to the top of the backlog
#47 remains at "maybe later"

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