Chris S
Chris S

Reputation: 87

C# Singleton - Basic Implementation and strategy

New C# devloper and programmer in general. I am hoping to gain insight into the overall proper usage of a Singleton, with insight on properly accessing class items. I cannot seem to figure out how this is done properly.

Background:

I am developing a C# WPF Application, employing MVVM.

The goal of my Singleton is to provide a globally-accessible class, where I am able to update the values inside within my Model.

This Singleton is called out at the top of my ViewModel, using the following syntax:

public CurrentMDL_Singleton currentMDL_Singleton = CurrentMDL_Singleton.Instance;

One of my ViewModels contains the following class:

public class CurrentMDL_Singleton
{
    private static CurrentMDL_Singleton instance;

    private CurrentMDL_Singleton()
    {
        CurrentMDL_Constructor currentMDL_Constructor = new CurrentMDL_Constructor();
    }

    public static CurrentMDL_Singleton Instance
    {
        get
        {
            if(instance == null)
            {
                //Create a new instance
                instance = new CurrentMDL_Singleton();
            }
            return instance;
        }
    }
}

Knowledge Check:

I intend to use this Singleton to create an instance of "currentMDL_Constructor". currentMDL_Constructor is another class which exists within the same ViewModel, seen below:

public class CurrentMDL_Constructor
{
    public Microsoft.Office.Interop.Excel.Application CurrentMDL_Excel { get; set; }
    public Microsoft.Office.Interop.Excel.Workbook CurrentMDL_Workbook { get; set; }
    public Microsoft.Office.Interop.Excel.Worksheet CurrentMDL_Worksheet { get; set; }
    public Microsoft.Office.Interop.Excel.Range CurrentMDL_xlRange { get; set; }
}

Problems:

I am unable to access the currentMDL_Constructor instance, created by the Singleton, as the Singleton constructor is a private member (understandable given the Singleton's purpose).

Within my Model, I am trying to tap into currentMDL_Constructor, to conduct something like the following:

CurrentMDL_Excel = new Microsoft.Office.Interop.Excel.Application();
CurrentMDL_Workbook = CurrentMDL_Excel.Workbooks.Open(MainWindow.MDL_Compare_VM.CurrentMDL_File);

I am unable to access currentMDL_Constructor to begin even thinking about having access to my CurrentMDL_Constructor class.

What is it that I am not understanding? I feel like I am getting lost in the world of instantiation, nested classes and a lack of basic knowledge with Singleton usage, and object orientation in general.

Thank you in advance, and I apologize if this post is redundant (could not find one that helped me).

-Chris

Upvotes: 0

Views: 298

Answers (2)

Chris S
Chris S

Reputation: 87

Thanks to all who helped with this. I was able to figure out where my problems mostly originated from, which was with the layout of the Singleton class.

I want to mention my reasoning for using this Singleton class. I need to ensure only one instance of these Excel variables exists. This is important because my ViewModel and Model depend on referencing instances of the same items. I have been struggling with closing these Excel files because I was mixing up instances. These Excel files are very large, need to remain open in between application functions and close directly after the second function. The Singleton design model solves the problem of ensuing a single instance, and also allows me to access the instance from within my Model very effortlessly.

Knowledge Check : Where I Went Wrong:

My previous understanding was that the private constructor within the Singleton class would set up what I would need to reference after the Singleton was instantiated. This was the wrong thinking.

Don't use the private constructor to instantiate or reference anything; this goes against the main use of the Singleton class. All we're trying to do is make sure only one instance of this Singleton exists; the pivate constructor is there to solely create the Singleton Instance to reference later.

The Fix :

Remove "CurrentMDL_Constructor" class, pull the items that were in that class, which I was trying to create a reference for, into the Singleton main class.

public class CurrentMDL_Singleton
{
    public Microsoft.Office.Interop.Excel.Application CurrentMDL_Excel { get; set; }
    public Microsoft.Office.Interop.Excel.Workbook CurrentMDL_Workbook { get; set; }
    public Microsoft.Office.Interop.Excel.Worksheet CurrentMDL_Worksheet { get; set; }
    public Microsoft.Office.Interop.Excel.Range CurrentMDL_xlRange { get; set; }

    private static CurrentMDL_Singleton instance = null;

    private CurrentMDL_Singleton() {}

    public static CurrentMDL_Singleton Instance
    {
        get
        {
            if(instance == null)
            {
                //Create a new instance
                instance = new CurrentMDL_Singleton();
            }
            return instance;
        }
    }
}

CurrentMDL_Excel, CurrentMDL_Workbook, CurrentMDL_Worksheet, etc. are now all accessible through calling out the instance within the Singleton (thank god). Within my Model, I'm now able to to the following:

> CurrentMDL_Singleton.Instance.CurrentMDL_Excel = new Microsoft.Office.Interop.Excel.Application();

My next step is making this Singleton thread-safe.

Thanks again for the help, and I hope this helps someone else who is trying to understand how to implement Singletons.

-Chris

Upvotes: 0

Georg
Georg

Reputation: 5771

Your code has multiple issues:

First, when writing a singleton that has no dependencies to other classes like in your case, it is best practise to have a static readonly field that holds the single instance, but you should instantiate it straight away like so:

private static CurrentMDL_Singleton instance = new CurrentMDL_Singleton();

This is because, whenever you reference a singleton class, you will probably use its instance. Moving the instantiation to a field init moves it into the type loader and therefore makes it thread-safe. In your code, if clients access the Instance property in parallel, you may end up in a race condition and hence with two different instances of a singleton class, which is something that you would typically want to avoid.

Second, your constructor creates an object and does not save it anywhere, hence you can't access it.

Third, there is absolutely no need to have an instance variable holding a singleton instance, because you can always get it from the static instance field. In your case, it is even dangerous because someone could change it to null and you would not notice.

Lastly, you should really reconsider whether it is really a singleton that you need. A singleton pattern is applied to make sure that there is only one instance of a class, usually not because it is easier to query its contents, because it is a dependency that you cannot easily exchange.

Upvotes: 3

Related Questions