robotlegs / robotlegs-framework

An ActionScript 3 application framework for Flash and Flex
https://robotlegs.tenderapp.com/
MIT License
967 stars 261 forks source link

IInjector #136

Closed creynders closed 11 years ago

creynders commented 11 years ago

So, what do you think? If we're adding it, it should be now, I think. See tschneidereit/SwiftSuspenders#88 And also http://knowledge.robotlegs.org/discussions/robotlegs-2/2124-mapping-multiple-interfaces-to-same-singleton-instance

creynders commented 11 years ago

@darscan Have you thought about this?

My vote is :+1: for IInjector.

creynders commented 11 years ago

I have a habit of stating the obvious.

creynders commented 11 years ago

It's not a problem, except for the pain in my knee each time I kick in an open door.

creynders commented 11 years ago

I'm just worried I'll need a wheel chair when I grow old.

creynders commented 11 years ago

Older, I mean, older.

darscan commented 11 years ago

Ha! Yes, it would be nice, but I seem to remember trying to create an IInjector in the framework api package and failing. Feel free to give it a "bash"

creynders commented 11 years ago

As a devout pastafarian I have bolognetic amounts of faith. His noodly appendages will guide my brain towards a solution. I think. I'm having a go.

On Sat, Jun 1, 2013 at 4:42 PM, Shaun Smith notifications@github.comwrote:

Ha! Yes, it would be nice, but I seem to remember trying to create an IInjector in the framework api package and failing. Feel free to give it a "bash"

— Reply to this email directly or view it on GitHubhttps://github.com/robotlegs/robotlegs-framework/issues/136#issuecomment-18790448 .

creynders commented 11 years ago

I feel a bit weird today, maybe I should eat something.

On Sat, Jun 1, 2013 at 5:49 PM, Camille Reynders <camille.reynders@gmail.com

wrote:

As a devout pastafarian I have bolognetic amounts of faith. His noodly appendages will guide my brain towards a solution. I think. I'm having a go.

On Sat, Jun 1, 2013 at 4:42 PM, Shaun Smith notifications@github.comwrote:

Ha! Yes, it would be nice, but I seem to remember trying to create an IInjector in the framework api package and failing. Feel free to give it a "bash"

— Reply to this email directly or view it on GitHubhttps://github.com/robotlegs/robotlegs-framework/issues/136#issuecomment-18790448 .

darscan commented 11 years ago

Good lord man! Have a salad.. The pasta will surely exacerbate your condition.

On 1 Jun 2013, at 16:49, creynders notifications@github.com wrote:

I feel a bit weird today, maybe I should eat something.

On Sat, Jun 1, 2013 at 5:49 PM, Camille Reynders <camille.reynders@gmail.com

wrote:

As a devout pastafarian I have bolognetic amounts of faith. His noodly appendages will guide my brain towards a solution. I think. I'm having a go.

On Sat, Jun 1, 2013 at 4:42 PM, Shaun Smith notifications@github.comwrote:

Ha! Yes, it would be nice, but I seem to remember trying to create an IInjector in the framework api package and failing. Feel free to give it a "bash"

— Reply to this email directly or view it on GitHubhttps://github.com/robotlegs/robotlegs-framework/issues/136#issuecomment-18790448 .

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

creynders commented 11 years ago

It works

But obviously it's a bit hum-hum, since I had to rename createChildInjector to createChild and parentInjector to parent and I had to upcast.

Another (probably the better) option would be to fork SS and do it there. I think that might be the best approach anyhow, to rely on our own fork.

darscan commented 11 years ago

Holy crap, you updated all the call sites!

I think we should fork Swiftsuspenders.. Till hinted at this a while back. Well, he mentioned incorporating Swiftsuspenders into the Robotlegs organisation. Now's as good a time as any.

darscan commented 11 years ago

It's just going to be awkward when people try to use the "official" swiftsuspenders SWC with RL..

creynders commented 11 years ago

Holy crap, you updated all the call sites!

Well of course.

I think we should fork Swiftsuspenders..

Yeps.

It's just going to be awkward when people try to use the "official" swiftsuspenders SWC with RL

There's not a lot of reason to do that, is there? Obviously we'll have to make it clear we're using a fork.

darscan commented 11 years ago

Hmm... I'm having serious second thoughts about this.. Perhaps we should just rename SwiftSuspendersInjector to RobotlegsInjector and leave it where it is. It's very weird forking an anti-IPrefix library purely to introduce an IPrefixed interface, just because we had a vote and (begrudgingly) decided to keep RL IPrefixed.

creynders commented 11 years ago

Yeah, but it allows us to introduce changes when necessary. That's my main motivation for forking anyway. Heheh, the epic i-prefix thread... :)

Were there call sites you wanted to leave as is, if possible?

On Sun, Jun 2, 2013 at 8:57 PM, Shaun Smith notifications@github.comwrote:

Hmm... I'm having serious second thoughts about this.. Perhaps we should just rename SwiftSuspendersInjector to RobotlegsInjector and leave it where it is. It's very weird forking an anti-IPrefix library purely to introduce an IPrefixed interface, just because we had a vote and (begrudgingly) decided to keep RL IPrefixed.

— Reply to this email directly or view it on GitHubhttps://github.com/robotlegs/robotlegs-framework/issues/136#issuecomment-18811611 .

darscan commented 11 years ago

From a maintenance and feature development perspective I'd still like to bring Swiftsuspenders into the Robotlegs organisation. But, it's still a stand-alone library, and I wouldn't want to apply changes purely for some aesthetic benefit to the Robotlegs framework.

Looking at what you've done here, I quite like the fact that the IInjector interface is part of the Robotlegs API. I'm going to rename the impl to RobotlegsInjector and push.

darscan commented 11 years ago

So, with this change, the context maps the injector instance as IInjector. This is going to break any existing (external) code that expects Injector. Do you think we should map both? Or, just loudly communicate this change and get everyone to change their code? Or, is it worth mapping both for convenience?

creynders commented 11 years ago

I'd go for loud!

Or, is it worth mapping both for convenience?

Speaking of which, shall I add some syntactic sucrose to map multiple types to one?

darscan commented 11 years ago

Speaking of which, shall I add some syntactic sucrose to map multiple types to one?

Ah yes, I had forgotten about that..

creynders commented 11 years ago

So, I started sketching out the syntactic sugar:

//--( setup )--//
injector.map(IReadOnly).toSingleton(SomeModel);

//--( API sketches)--//
//1. ugly shizzle - possible
injector.reuseRuleOf(IReadOnly)
    .forType(IWriteOnly);

//2. so-so - problematic
injector.map(IWriteOnly)
    .toRuleOf(IReadOnly);

//3. my preference - problematic
injector.map(IReadOnly)
    .and(IWriteOnly)
    .toSingleton(SomeModel);

//4. idiotic, but most "honest" - possible
injector.useSingletonOf(IReadOnly)
    .with(IWriteOnly);

//5. maybe most pragmatic solution - possible
const provider:DependencyProvider = injector.getProvider(IReadOnly);
injector.map(IWriteOnly).toProvider(provider);

If I'm not mistaken the only real use case for this is when you want to map multiple types to the same singleton, right? Either for a toSingleton or a asSingleton. That's why I called 4/ the most honest one, but it's definitely no good.

I'd prefer 3/ because it's the most readable, but obviously it only solves a part of the problem, since you'd still need a separate sugary method to reuse a provider outside the mapping chain.

With 'problematic' I mean we'd need to modify SS. There's simply no way around it. map returns a concrete InjectionMapping type which is declared in SS. If I would want to change the return type of map in IInjector I'd need to rename the method, which is simply not an option IMO. Obviously I could choose to have RobotlegsInjector not extend Injector (thus get full control of the IInjector API) but that road deteriorates fast. You'd create a wrapper instance, the whole createChild and parent stuff gets a lot more complicated, etc. I tried it out when I created the IInjector. This is kind of the reason why I would've opted for maintaining our own SS fork (until/unless it's donated to RL).

I'm leaning towards 5/ but maybe that's just because I'm not satisfied with any of the other method names. But 5/ is a bit opaque IMO. But I need to check whether it's really possible. (Definitely possible, it was 3/ I wasn't sure of)

darscan commented 11 years ago

So, I reckon that this needs to be solved at the library level (and not in our adapter). I'd prefer to keep the adapter interface 1-1 with the Swiftsuspenders API (where possible).

Perhaps we should put this on hold until we've brought the repo into our org?

creynders commented 11 years ago

I'd rather opt for the getProvider solution, and go 1-to-1 when possible, no? Don't get me wrong, I understand your hesitation.

On Tuesday, June 4, 2013, Shaun Smith wrote:

So, I reckon that this needs to be solved at the library level (and not in our adapter). I'd prefer to keep the adapter interface 1-1 with the Swiftsuspenders API (where possible).

Perhaps we should put this on hold until we've brought the repo into our org?

— Reply to this email directly or view it on GitHubhttps://github.com/robotlegs/robotlegs-framework/issues/136#issuecomment-18911647 .

Sent from Gmail Mobile

darscan commented 11 years ago

I see that getProvider is a pre-existing internal method that expects a mappingId string. Perhaps we can add getProviderFor that accepts a type and name?

creynders commented 11 years ago

Pushed it to reuse_rule But if you want to wait to get SS into RL, np!

creynders commented 11 years ago

created dedicated issue for IInjector#getProviderFor

neilmanuell commented 10 years ago

ha! been creating some SSInjector dependent utils and now wamt to use in RL2. Ba! injecting to Injector not IInjector

face palm

neilmanuell commented 10 years ago

actually, all I have to do is to map Injector to Injector... mmm