Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

What's wrong with this singleton I created

I created a class which allows access to global access to a variable while only creating it once, essentially a singleton.

However, it doesn't match any of the 'correct' ways to implement a singleton. I assume that it's not mentioned because there is something 'wrong' with it but I can't see any problem with it other then the lack of lazy initialization.

Any thoughts?

static class DefaultFields
{
    private static readonly string IniPath = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "defaultFields.ini");
    private static readonly IniConfigSource Ini = GetIni();               

    /// <summary>
    /// Creates a reference to the ini file on startup
    /// </summary>
    private static IniConfigSource GetIni()
    {
        // Create Ini File if it does not exist
        if (!File.Exists(IniPath))
        {
            using (FileStream stream = new FileStream(IniPath, FileMode.CreateNew))
            {
                var iniConfig = new IniConfigSource(stream);
                iniConfig.AddConfig("default");
                iniConfig.Save(IniPath);
            }
        }

        var source = new IniConfigSource(IniPath);
        return source;
    }

    public static IConfig Get()
    {
        return Ini.Configs["default"];
    }

    public static void Remove(string key)
    {
        Get().Remove(key);
        Ini.Save();
    }

    public static void Set(string key, string value)
    {
        Get().Set(key, value ?? "");
        Ini.Save();
    }
}
like image 533
John Avatar asked Dec 10 '22 14:12

John


2 Answers

It doesn't follow the usual singleton patterns as your class is static and just controls access to static variables.

Where as a singleton is normally a static single instance of a class where the only static functions are that to create and access the singleton that stores variables as normal non static member variables.

Meaning the class could quite easily be changed or made to be instanced more then once but yours cannot

like image 177
Tristan Avatar answered Dec 23 '22 11:12

Tristan


You are right about the singleton, its a class with a unique instance that provides global access.

It might seem like a static class but usually its implemented in a different way.

Also bear in mind that this pattern should be used with some precaution, as its really hard to refactor out a singleton once its deep in the code. Should be used primarily when you have hardware constraints or are implemented unique points of access to a factory. I would try to avoid it whenever possible.

An example of an implementation is as follows:

public class A
{
    /// <summary>
    /// Unique instance to access to object A
    /// </summary>
    public static readonly A Singleton = new A();

    /// <summary>
    /// private constructor so it can only be created internally.
    /// </summary>
    private A()
    {
    }

    /// <summary>
    /// Instance method B does B..
    /// </summary>
    public void B()
    {
    }
}

And could be used like

A.Singleton.B()

Hope that helps.

like image 39
Nuno Ramiro Avatar answered Dec 23 '22 12:12

Nuno Ramiro