Copied to clipboard

Flag this post as spam?

This post will be reported to the moderators as potential spam to be looked at


  • Lee 1117 posts 3051 karma points
    Oct 16, 2019 @ 14:52
    Lee
    0

    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.Current or UmbracoContext.Current

    And 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!?

        // 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)
        );
    

    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?

  • Matt Brailsford 2284 posts 11100 karma points MVP 6x c-trib
    Oct 16, 2019 @ 15:12
    Matt Brailsford
    1

    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.

  • Lee 1117 posts 3051 karma points
    Oct 16, 2019 @ 15:20
    Lee
    0

    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.

  • Matt Brailsford 2284 posts 11100 karma points MVP 6x c-trib
    Oct 16, 2019 @ 15:24
    Matt Brailsford
    0

    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. 🤷‍♂️

  • Lee Kelleher 3884 posts 14615 karma points MVP 9x admin c-trib
    Oct 16, 2019 @ 15:48
    Lee Kelleher
    1

    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.

Please Sign in or register to post replies

Write your reply to:

Draft