And even in the docs for setting up Unity IOC in Umbraco, it states to use the singleton!?
// The UmbracoContext must be registered so that the Umbraco backoffice controllers
// can be successfully resolved.
container.RegisterType<UmbracoContext>(
new PerRequestLifetimeManager(),
new InjectionFactory(c => UmbracoContext.Current)
);
In general, I believe it's considered bad practice to use singletons as they aren't testable or extendable. We can't fully get rid of this right now in the codebase, so it's my understanding that these are being exposed via these base properties to allow the core to hide how it is accessing those services.
Yes, right now it's using the singletons, but I'd imagine this would want to change later to access them from another location, such as DI.
By using the singletons directly, you then tie your code to the singleton implementation and this swap out becomes pretty hard as now you need to replace all places you are using the singleton. By using the base properties, you wouldn't need to.
Now you say that, that does ring a bell someone mentioning that, but I can't see how that would be. Maybe someone else can add some additional light on it. 🤷♂️
I assumed the discouragement of using the Singletons was more about "best practices" and "cleaner code" (in terms of unit-testing, etc) ... and to encourage the use of inheriting from Umbraco's base classes.
I haven't seen any concrete evidence of memory leaks. But then I have no evidence that they don't.
Confused about singleton usage in v7 (i.e. UmbracoContext.Current) and why it's bad practice?
I was reading through the Common Pitfalls page and noticed it warns against using the singletons?
ApplicationContext.CurrentorUmbracoContext.CurrentAnd instead says that you have to use the exposed properties on the SurfaceController or UmbracoApiController etc...
However, if you did into the Umbraco codebase. All these exposed properties actually use the singleton anyway! Example of SurfaceController below:
https://github.com/umbraco/Umbraco-CMS/blob/master-v7/src/Umbraco.Web/Mvc/SurfaceController.cs#L37
And even in the docs for setting up Unity IOC in Umbraco, it states to use the singleton!?
https://our.umbraco.com/Documentation/Reference/Using-Ioc/index-v7
So I'm confused about the warnings and why they are listed as pitfalls? When in fact Umbraco uses these singletons behind the scenes?
In general, I believe it's considered bad practice to use singletons as they aren't testable or extendable. We can't fully get rid of this right now in the codebase, so it's my understanding that these are being exposed via these base properties to allow the core to hide how it is accessing those services.
Yes, right now it's using the singletons, but I'd imagine this would want to change later to access them from another location, such as DI.
By using the singletons directly, you then tie your code to the singleton implementation and this swap out becomes pretty hard as now you need to replace all places you are using the singleton. By using the base properties, you wouldn't need to.
Ok that's what I assumed. It was just in the pitfall section and I thought there was an issue with memory leaks or something.
Now you say that, that does ring a bell someone mentioning that, but I can't see how that would be. Maybe someone else can add some additional light on it. 🤷♂️
I assumed the discouragement of using the Singletons was more about "best practices" and "cleaner code" (in terms of unit-testing, etc) ... and to encourage the use of inheriting from Umbraco's base classes.
I haven't seen any concrete evidence of memory leaks. But then I have no evidence that they don't.
is working on a reply...