Skip to content
This repository has been archived by the owner on Jul 3, 2019. It is now read-only.

Cannot instantiate an Interface - even if there is a Rule for it #15

Closed
darscan opened this issue Dec 14, 2009 · 11 comments
Closed

Cannot instantiate an Interface - even if there is a Rule for it #15

darscan opened this issue Dec 14, 2009 · 11 comments

Comments

@darscan
Copy link
Contributor

darscan commented Dec 14, 2009

I kinda suspected that this wouldn't work:

    injector.mapClass(IVehicle, Car);
    var vehicle:IVehicle = injector.instantiate(IVehicle);

but don't you think it should?

@tschneidereit
Copy link
Owner

Ooh, I like that idea! Will definitely integrate it into the next release.

I guess the behavior should be as follows:
Input: Interface for which the injector has a mapClass rule | Output: instance of the mapped class
Input: Interface for which the injector doesn't have any rule mapped | Output: throw InjectionError "no class mapping found while trying to instantiate interface X"
Input: Interface for which the injector has a mapValue, mapSingleton or mapSingletonOf rule | Output: throw InjectionError "can't instantiate current mapping for interface X"

Thoughts on these results?

@darscan
Copy link
Contributor Author

darscan commented Dec 14, 2009

I agree with:

Input: Interface for which the injector doesn't have any rule mapped | Output: throw InjectionError "no class mapping found while trying to instantiate interface X"

But I think if any rule exists, it should return the appropriate instance. I.E:

injector.SingletonOf(IVehicle, Car);
var vehicle:IVehicle = injector.instantiate(IVehicle);

should return the singleton Car instance - regardless of whether or not that instance already exists. It should be used to retrieve a dependency of a given type. Or, put another way:

instantiate() might have been better named as getOrCreateInstance().

What do you think? (obviously, I'm not suggesting changing the name, just using that to illustrate the point).

@tschneidereit
Copy link
Owner

Mmh. While I see the utility of being able to get the result of any mapping, I really think that that functionality shouldn't be provided by a function named "instantiate".

What about
applyMapping(requestedType : Class, named : String = null) : *
or
getMappingResult(requestType : Class, named : String = null) : *
?

@darscan
Copy link
Contributor Author

darscan commented Dec 14, 2009

I totally agree that the name doesn't fit with the functionality. Though I'm not sure that the proposed names are better. I think it's over complicating things to refer to the mappings (an implementation detail).

If we go back to the beginning, and think about what an IoC container is, then the desired functionality is simply about getting an instance from the container - without knowing anything about the rules that might create/find such an instance. It shouldn't matter whether the rules generate a new instance or re-use an old one - we shouldn't care/know about that from the outside. We just want an instance.

@tschneidereit
Copy link
Owner

Yup, that's how see it as well. But it seems to me that this means we really can't use instantiate for this task - both because it's name is wrong and because it doesn't accept a 'named' parameter, which is essential for being able to get instances for all mapped rules.

How about adding getInstance(forClass : Class, named : String = null) : * and marking instantiate as deprecated? I might even remove it from SwiftSuspenders in a 2.0 release and only add it to the RL adapter.

@darscan
Copy link
Contributor Author

darscan commented Dec 14, 2009

I think that's a brilliant plan!

@tschneidereit
Copy link
Owner

Cool - I'll implement it in the next few days and roll it into the 1.5 release.

@darscan
Copy link
Contributor Author

darscan commented Dec 14, 2009

Awesome =) No rush from my side - was just trying to put together a little DI tutorial, and this popped up.

@tschneidereit
Copy link
Owner

getInstance is in 1.5.0b4

@tschneidereit
Copy link
Owner

And now to actually close this issue ...

@texastoland
Copy link
Contributor

I missed this when I originally asked about it on the forum. Great insight guys also for deferring deprecating #instantiate :)

This issue was closed.
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants