How to implement the Singleton class for defining system variables?

Question

I am trying to implement the Singleton Pattern.

Here is what I have done so far:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using MySql.Data.MySqlClient;
namespace POS
{
    public sealed class Settings
    {
        private static Settings instance = null;
        private static readonly object padlock = new object();

        //System Variables
        public string SYSTEM_NAME { get; set; }

        public Settings()
        {
            //start new connection to the database
            dbConnetion db = new dbConnetion();

            string sql = " SELECT system_name "
                         + " FROM configuration "
                         + " LIMIT 1";

            //read the system variable
            foreach (var i in db.getData(sql, null, r =>
                                                new globalSettings()
                                                {
                                                    _sys_name = r["system_name"].ToString()
                                                }
                                         )
                    )
            {
                SYSTEM_NAME = i._sys_name;
            }
        }

        public static Settings Instance
        {
            get
            {
                lock (padlock)
                {
                    if (instance == null)
                    {
                        instance = new Settings();
                    }
                    return instance;
                }
            }
        }
    } 


    class globalSettings
    {
        public string _sys_name;
    }

 }

And when I want to read the variable I do this:

GlobalName.Text = Settings.SYSTEM_NAME;

but this is not working. I get the following error:

Error 1 An object reference is required for the non-static field, method, or property 'POS.Settings.SYSTEM_NAME.get'

How can I read the variables correctly? also did I manage to implement the Singleton pattern correctly?

Answer 1

In your code, you want:

GlobalName.Text = Settings.Instance.SYSTEM_NAME;

this will instantiate the singleton if it doesn't exist, and allow you to use its instance (stored in the Instance static property).

For storing such properties there is an easier pattern that can be used in C#:

public static class Settings
{
    public static string SystemName { get; set; }

    public const string SomeOtherProperty = "x";

    public static int AnotherOne
    {
        get
        {
            return 42;
        }
    }
}

Be warned however that such global objects introduce a hidden dependency, making your code harder to maintain (changes in one place may affect distant, non-obvious places in code. - less apparent if all properties are constants though) and harder to test (you can't stub/mock a singleton).

To make your implementation a bit cleaner:

1. Your class violates the Single Responsibility Principle: you should separate reading the configuration from DB from the object that simply aggregates it.

2. Make the dependency on the configuration explicit. For example, each class that needs the configuration should accept its instance in the constructor and use it for its purposes. This way you can also easily test such class, providing different configurations and testing behaviour for different settings.

Then you can test your classes without using the database, but using some hardcoded settings. Imagine also, that you may want to read the settings from a file or a command line sometimes.