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

Add syntactic sugar for mapping a class to itself #2

Open
creynders opened this issue Aug 8, 2013 · 15 comments
Open

Add syntactic sugar for mapping a class to itself #2

creynders opened this issue Aug 8, 2013 · 15 comments

Comments

@creynders
Copy link
Member

ATM if you want to map a class to be instantiated afresh each time you need to do:

injector.map(FooClass).toType(FooClass);

Maybe it would be a good idea to add this to the API:

injector.map(FooClass).asType();

It's in symmetry with

injector.map(FooClass).asSingleton();

@darscan What ya think?

@creynders
Copy link
Member Author

@darscan
Copy link
Member

darscan commented Aug 8, 2013

Can't you just do this: injector.map(FooClass);

@creynders
Copy link
Member Author

Ummm... Do you mean that is how it already works? Or as a suggestion of how it could work?
If the former, I didn't realize that.

@creynders
Copy link
Member Author

It does break the symmetry, no? And it's not really clear either.

@darscan
Copy link
Member

darscan commented Aug 8, 2013

I believe that's how it currently works. I'm not fond of it myself.

@creynders
Copy link
Member Author

Ok, I'll test it out. Would you agree on adding InjectionMapping#asType for clarity (and obsessive compulsive disorder soothing medicine)?

@darscan
Copy link
Member

darscan commented Aug 8, 2013

Sounds good to me :)

@tschneidereit
Copy link

That's exactly how it already works. But I agree: adding
InjectionMapping#asType makes a lot of sense, if only for symmetry's sake.

@creynders
Copy link
Member Author

🤘

@Stray
Copy link
Member

Stray commented Aug 8, 2013

wades in

is there something better than: asType() ?

'asType' always bothered me - because when do you map something not as a type? So it kind of means "not special", which is a very icky bit of venn diagram labelling.

IIRC, it means newInstanceEachTimeFreshlyConstructedByTheAppropriateInjector() ... which obviously sucks, but perhaps there is something both pithy and less opaque than 'asType'?

Sx

On 8 Aug 2013, at 13:00, creynders notifications@github.com wrote:


Reply to this email directly or view it on GitHub.

@creynders
Copy link
Member Author

@Stray Yeez. Unfortunately you're quite right.

@creynders
Copy link
Member Author

asNothingSpecial

@creynders
Copy link
Member Author

asNotSingleton

@creynders
Copy link
Member Author

@tschneidereit I just took a quick look and was wondering why exactly it's done like this in the InjectionMapping constructor?

_defaultProviderSet = true;
mapProvider(new ClassProvider(type));

Seems a bit wasteful to have a ClassProvider instance created for each mapping, only to be thrown away in 90% of the cases.
Was it for robustness? Making sure nothing explodes if someone writes injector.map(FooClass) and forgets to map it to something.
What happens with interfaces though? And wouldn't it be preferable to have it explode?

@tschneidereit
Copy link

@tschneidereit https://github.com/tschneidereit I just took a quick look
and was wondering why exactly it's done like this in the
InjectionMappingconstructor?

_defaultProviderSet = true;mapProvider(new ClassProvider(type));

Seems a bit wasteful to have a ClassProvider instance created for each
mapping, only to be thrown away in 90% of the cases.
Was it for robustness? Making sure nothing explodes if someone writes
injector.map(FooClass) and forgets to map it to something.
What happens with interfaces though? And wouldn't it be preferable to have
it explode?

It will explode for interface, but later. I agree that this is a bit
unfortunate.

I went back and forth on this quite a bit. In the end, I decided that I'd
roughly go with what other frameworks do, and make using instance-factory
providers easy. See, it doesn't look like this will be thrown away 90% of
the time. That, combined with ClassProvider being very cheap and
injection mapping usually not being on hot paths made me go with this
setup. If I remember everything correctly, that is.

Another consideration is that, without setting a default, it's pretty easy
to leave mappings in an unfinished state, in which case they'll blow up
later. Whether that is worse than automatically, and perhaps unexpectedly,
injecting new instances, I don't know.

@darscan was talking about the ability to "finish" an injector, making it
impossible to add more mappings, or change existing ones. Besides
potentially enabling some optimizations, that would make it trivial to
check for unfinished mappings and throw for them. A weaker version of this
would be to introduce a verifyMappings method in Injector, to be called
when one assumes all mappings to be complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants