Avoid non-readonly static fields - Immutability NDepend

Question

I am using NDepend for code analysis and got this warning:

https://www.ndepend.com/default-rules/NDepend-Rules-Explorer.html?ruleid=ND1901#!

This rule warns about static fields that are not declared as read-only.

In Object-Oriented-Programming the natural artifact to hold states that can be modified is instance fields. Such mutable static fields create confusion about the expected state at runtime and impairs the code testability since the same mutable state is re-used for each test.

My code is as follows:

using Cosmonaut;
using Microsoft.Azure.Documents.Client;
using System.Configuration;
using LuloWebApi.Entities;


namespace LuloWebApi.Components
{
    /// <summary>
    /// Main class that encapsulates the creation of instances to connecto to Cosmos DB
    /// </summary>
    public sealed class CosmosStoreHolder
    {
        /// <summary>
        /// Property to be initiated only once in the constructor (singleton)
        /// </summary>
        private static CosmosStoreHolder instance = null;

        /// <summary>
        /// To block multiple instance creation
        /// </summary>
        private static readonly object padlock = new object();

        /// <summary>
        /// CosmosStore object  to get tenants information
        /// </summary>
        public Cosmonaut.ICosmosStore<SharepointTenant> CosmosStoreTenant { get; }

        /// <summary>
        /// CosmosStore object  to get site collection information
        /// </summary>
        public Cosmonaut.ICosmosStore<SiteCollection> CosmosStoreSiteCollection { get; }

        /// <summary>
        /// CosmosStore object  to get page templates information
        /// </summary>
        public Cosmonaut.ICosmosStore<PageTemplate> CosmosStorePageTemplate { get; }

        /// <summary>
        /// CosmosStore object  to get pages information
        /// </summary>
        public Cosmonaut.ICosmosStore<Page> CosmosStorePage { get; }

        /// <summary>
        /// CosmosStore object  to get roles information
        /// </summary>
        public Cosmonaut.ICosmosStore<Role> CosmosStoreRole { get; }

        /// <summary>
        /// CosmosStore object  to get clients information
        /// </summary>
        public Cosmonaut.ICosmosStore<Client> CosmosStoreClient { get; }

        /// <summary>
        /// CosmosStore object  to get users information
        /// </summary>
        public Cosmonaut.ICosmosStore<User> CosmosStoreUser { get; }

        /// <summary>
        /// CosmosStore object  to get partners information
        /// </summary>
        public Cosmonaut.ICosmosStore<Partner> CosmosStorePartner { get; }

        /// <summary>
        /// CosmosStore object  to get super administrators information
        /// </summary>
        public Cosmonaut.ICosmosStore<SuperAdministrator> CosmosStoreSuperAdministrator { get; }

        /// <summary>
        /// Constructor
        /// </summary>
        CosmosStoreHolder()
        {

            CosmosStoreSettings settings = new Cosmonaut.CosmosStoreSettings(ConfigurationManager.AppSettings["database"].ToString(),
                 ConfigurationManager.AppSettings["endpoint"].ToString(),
                 ConfigurationManager.AppSettings["authKey"].ToString());

            settings.ConnectionPolicy = new ConnectionPolicy
            {
                ConnectionMode = ConnectionMode.Direct,
                ConnectionProtocol = Protocol.Tcp
            };


            CosmosStoreTenant = new CosmosStore<SharepointTenant>(settings);
            CosmosStoreSiteCollection = new CosmosStore<SiteCollection>(settings);
            CosmosStorePageTemplate = new CosmosStore<PageTemplate>(settings);
            CosmosStorePage = new CosmosStore<Page>(settings);
            CosmosStoreRole = new CosmosStore<Role>(settings);
            CosmosStoreClient = new CosmosStore<Client>(settings);
            CosmosStoreUser = new CosmosStore<User>(settings);
            CosmosStorePartner = new CosmosStore<Partner>(settings);
            CosmosStoreSuperAdministrator = new CosmosStore<SuperAdministrator>(settings);
        }

        /// <summary>
        /// Instance access, singleton
        /// </summary>
        public static CosmosStoreHolder Instance
        {
            get
            {
                lock (padlock)
                {
                    if (instance == null)
                    {
                        instance = new CosmosStoreHolder();
                    }
                    return instance;
                }
            }
        }
    }
}

However I am not sure how to fix this warning.

Answer 1

This is a guide, not a hard rule. Usually, non-readonly static fields are hard to intuit about. But in this case you're doing lazy deferred loading, so... a lock and mutate is indeed one way of achieving that, without causing it to be loaded prematurely.

So a pragmatic fix is: just ignore/override the warning

---

Another approach, however, is to move the field to another type where it is readonly, and rely on the deferred .cctor semantics:

public static CosmosStoreHolder Instance {
   [MethodImpl(MethodImplOptions.NoInlining)]
   get => DeferredHolder.Instance;
}

private static class DeferredHolder {
    internal static readonly CosmosStoreHolder Instance = new CosmosStoreHolder();
}

Then you don't even need the lock semantics (.cctor deals with that for you).