Is a singleton with an update method and software state verification a valid pattern?

Question

Stack overflow suggested that this question my be better suited here. I normally work in c#, but I'm working in a vb.net net application currently. So feel free to respond with vb.Net or c#...

Essentially I have a singleton that does all of the heavy lifting of getting global settings which are stored in the database; but there is an interface (dialog form) that allows those settings to change.

So what I did was create a singleton which on the first instantiation (only) runs the public update method. If, however, a user opens the settings dialog, the singleton is instantiated and the update command is invoked separately just in case the user made changes to the settings in the settings dialog... its the only place where I run the Update method outside of the initial instantiation of the singleton object. Conceivably another errant developer could irresponsibly run the Update method over and over again, but it would not harm anything except slow down the application--and this is the main reason I wanted to use a singleton, because there are cases where with calls to the settings is made hundreds if not thousands of times in loops; which would really slow down the application, and also the settings are being called from many random locations depending on what the user does... why don't I use a simple DTO, well, that's described below.

The other crazy thing about this application is that multiple "environments" can be running in the same thread, each with completely different settings (long story). The way the prior developers handled this was by making a call to update a settings DTO every time there was a possible switch or change in the settings or "environment", or upon starting up the application. On a search there are around 40 locations... seems nuts to me. This is on top of the hundreds of calls were the DTO is accessed. I want this singleton to do the heavy lifting and verify which environment the app is in and retrieve the settings only once from the database--except on that one occasion with the settings dialog I mentioned above, where the Update method is invoked.

End result, one nutty singleton... Below is a simplified version of the code. Basically, I'm asking whether is this a reasonable pattern, or is there a better one for this? Note that one potential downside to my solution is that I still need to make a database call, as a check to the current database, every time this class is instantiated. Thanks for the feedback.

   Public Class MySingleton

        Private Shared _Instance As MySingleton = Nothing

        Private _ExampleSetting As String

        Private _CurrentDatabase as DBDatabase

        Public Shared ReadOnly Property GetInstance() As MySingleton
            Get
                If _Instance Is Nothing Then
                      _Instance = New MySingleton()
                ElseIf _CurrentDatabase <> GetCurrentDataBase() Then 
                      _Instance = New MySingleton()
                End If

                Return _Instance
            End Get
        End Property

        Public ReadOnly Property ExampleSetting As String
            Get
                Return _ExampleSetting
            End Get
        End Property


        Private Sub New()

            Update()

        End Sub


        Public Sub Update()

            _CurrentDatabase  = GetCurrentDataBase()

            _ExampleSetting = _CurrentDatabase.GetSetting("exampleSetting")

        End Sub

    End Class

Answer 1

Your "singleton" isn't a singleton at all. It's a data cache, which can result in different parts of your app having different version of that cache. In C# code, as my VB.Net is weak:

var instance1 = MySingleton.GetInstance();
// assume instance1.ExampleSetting would return "foo"

// do something that changes ExampleSetting to bar, such that
// _CurrentDatabase <> GetCurrentDataBase() is now true

var instance2 = MySingleton.GetInstance(); // new instance
var x = instance1.ExampleSetting; // will be foo
var y = instance2.ExampleSetting; // will be bar

Just because you are overriding the static _Instance with a new instance won't affect references to the previous instances held by other parts of the code. You are relying on the developer calling MySingleton.GetInstance() every time to get the latest "singleton".

Update

To address the question of "is there a better way", one solution is to use dependency injection. Rather than have a globally accessible not-quite-singleton, instead create just one instance of MySingleton and pass it to all parts of the system that need it. Then, whenever it is queried, it can check if the database has changed and update itself, rather than spawning a new instance.

Answer 2

The essence of a Singleton is that There Can Be Only One. There is ONE instance of a class being created when it's needed the very first time, no other instance is ever created, and the one instance stays alive as long as the app is running. So what you have is not a singleton.

And it is absolutely legal and expected that some bit of code gets a reference to that Singleton once and uses that reference forever. So there is absolutely zero reason to believe that one piece of code will ever call your GetInstance () more than once.

It seems that your GetCurrentDatabase() is some global state that is modified in some uncontrolled way. All you managed to do was turn MySingleton () also into global state that is modified in some uncontrolled way. I'd say you want individual objects, each created by the code that wants to access data in different ways, and these objects can talk to one Singleton that optimises access as well as it can.

Answer 3

On closer inspection, as @David Arno has identified,that isnt a singleton. The value returned by GetInstance potentially varies on each call.

Additionally (and this could simply be my lack of knowledge re. VB semantics) the fact that you dont appear to have any restrictions on the use of the constructor for your singleton class means that clients are free to create their own instances.

Singleton has a well understood definition in software development circles. What you have isnt an 'odd singleton', its just not a singleton.

Theres nothing wrong with the internal state of the singleton changing (as long as theres only a single instance of it), so if your individual properties were to check whether or not a database existed, and determine the return value at that point, you could still retain singleton semantics. Whether or not you could make this perform at scale is a different question.