[Discussion]: AddInstance was renamed to AddSingleton

[pranavkm]

Discussion PR for #319

The rename makes the 3 APIs line up

AddSingleton<IService, MyService>();

AddSingleton<IService>(serviceProvider => new MyService(serviceProvider));

AddSingleton<IService>(new MyService());

[NeelBhatt]

👍

[pebezo]

https://docs.asp.net/en/latest/fundamentals/dependency-injection.html

One key difference between Instance services and Singleton services is that the Instance service is created in ConfigureServices, while the Singleton service is lazy-loaded the first time it is requested.

So if you "merge" or rename, how would the new functionality fit? How would I know where the instance is created?

[pranavkm]

That doesn't change. You're still passing in an instance so it's instantiated wherever you're create it.

[pebezo]

Duh, good point 😄

[matlus]

Should probably call is AddSingleInstance() instead?

[pranavkm]

@matlus, we don't differentiate between the factory method and the type overload (i.e. we don't have a AddSingletonFactory) and we haven't had any issues with that as yet. I'm not sure why the instance overload would warrant a different method name.

[matlus]

@pranavkm I don't see a Singleton needs a Factory (since there is only one).

AddInstance is clearer (to me) than AddSingleton in my honest opinion.

I guess the confusion is that "Singleton" is an overloaded term here. I assumed (initially) my class needed to be designed as a Singleton, but we're say "give me an instance and I'll just reuse that instance each time". so it's effectively a singleton but it is not designed to be.

Thinking back on it, I'd stick to AddInstance() :). code documentation in these methods will make the intent/operation clearer.

[Eilon]

A singleton could need a factory if it is expensive or complex to construct, yet might not be needed.

[pranavkm]

The factory lets you get at other services via the service container that is passed to it. We use this in a couple of places as a substitute for creating instance factories.

[matlus]

The factory lets you get at other services via the service container that is passed to it. We use this in a couple of places as a substitute for creating instance factories.

Right, that's exactly what I use the AddInstance() for now. But I create the instance and hand it off (obviously). So you're saying the overload that takes a Factory method, will use the Factory method just once? Or do I have to pass you a method that creates an instance where the class is designed to be a singleton?

Ok, looking at Eilon's answer makes sense from a lazy loading need point of view. I see the use for this functionality (obviously - since I'm using it 😃). I do still have a confusion with the name.

[davidfowl]

So you're saying the overload that takes a Factory method, will use the Factory method just once?

Yes it will be called just once, since it's a singleton.

Or do I have to pass you a method that creates an instance where the class is designed to be a singleton?

Yes, if you add a singleton the class should be designed to be a singleton.

Ok, looking at Eilon's answer makes sense from a lazy loading need point of view. I see the use for this functionality (obviously - since I'm using it 😃). I do still have a confusion with the name.

The delegate is not just for lazy loading, it's for getting at other registered services at time of construction:

services.AddSingleton<IFoo>(sp => new Foo(sp.GetRequiredService<IBar>());

At the time you call AddSingleton(instance) you may not have access to everything required to construct the instance.

[matlus]

I get the need/purpose of this method and its overloads. By the way does the DI container call Dispose for any instances it is managing and id the type implements IDisposable?

Yes, if you add a singleton the class should be designed to be a singleton.

Ok, I see why you've answered "yes" to both. However, I don't think I was clear when I said, "Designed to be a singleton".

Of course when you're sharing one instance across multiple requests you have to be careful about state and have to be thread-safe since multiple simultaneous request could be partying on your class at the same instant in time.

But does the class truly need to be designed to allow only one instance of you call the factory method exactly once even when multiple threads may be requesting this instance at the exact same time?

[Eilon]

@matlus only one instance will be created. The class must be thread-safe or otherwise provide some sort of thread safety guarantee (e.g. a very poor solution would be some kind of external synchronization, but that's an extreme anti-pattern).

The class does not need to require allowing only one instance. In fact, that is also an anti-pattern. E.g. in a unit test you might want to instantiate many instance of the class to test.

[sebastienros]

I see that the overload AddInstance(Type type, object instance) has been removed, not just renamed as expected. I am opening an issue as this is blocking me right now.

#366

[Eilon]

We are closing this issue because no further action is planned for this issue. If you still have any issues or questions, please log a new issue with any additional details that you have.