ipjohnson / Grace.DependencyInjection.Extensions

Grace Extensions for ASP.Net Core
19 stars 7 forks source link

Microsoft DI GetService consistency #8

Closed stephenlautier closed 6 years ago

stephenlautier commented 6 years ago

Since we switched to Grace, several Microsoft DI extensions seems a bit inconsistent when using GetService and GetRequiredService.

Basically GetService and GetRequiredService should work as the name suggests - GetRequiredService should always throw; GetService shouldnt throw.

What do you think?

ipjohnson commented 6 years ago

@stephenlautier can you create a simple example showing your use case. The reality is you are mixing to different ideas together in one case you are telling the container you specifically don't want it to throw an exception if you return null yet you want it to throw an exception when you return null.

ipjohnson commented 6 years ago

@stephenlautier In regards to your proposal:

I'll look into changing the behavior for AllowInstanceAndFactoryToReturnNull = true

stephenlautier commented 6 years ago

Yes, to be honest I wasn't 100% sure with the AllowInstanceAndFactoryToReturnNull = false and GetService whether it should throw or not. The reason i concluded that maybe it should be like that because if you would use a library which might use GetService internally and its optional, you will get different behaviors. Infect i had this issue, since we had one optional service in a specific case, so had to enable AllowInstanceAndFactoryToReturnNull.

So basically what i was then more in favor is to be as following

Just a suggestion ofc, but the main one was the GetRequiredService. Thanks!

ipjohnson commented 6 years ago

@stephenlautier for the moment I'm going to just address the GetRequiredService problem but at some point I'll probably add an AllowNull method for exporting factories and turn it on for factories coming from the service collection for compatibility sake.

I've pushed a beta to nuget with a null reference check for GetRequiredService let me know how it works

stephenlautier commented 6 years ago

@ipjohnson thats good enough for me, thanks!

ipjohnson commented 6 years ago

@stephenlautier did this take care of it for you?

stephenlautier commented 6 years ago

@ipjohnson yes it seems fine now. sorry for the late response