1

On a multithread application (ASP.NET MVC) I need to have a global settings class which contains constants and values taken from Web.Config.

I would like to have this class static, as singleton ... And locked?

public static class Settings {

  public static LoggerSettings Logger;
  public static MailerSettings Mailer;

  public class LoggerSettings {
    public String Levels { get { return ConfigurationManager.AppSettings["Logger.Levels"]; } }
    public const String Report = "team@xyz.com";
  } // LoggerSettings

  public class MailerSettings {
    public String Contact { get { return ConfigurationManager.AppSettings["Mailer.Contact"]; } }
  } // MailerSettings

}

I think I should implement a double lock? No?

I am not sure the best way to do this. Could I, please, get some help?

Thank You, Miguel

Miguel Moura
  • 36,732
  • 85
  • 259
  • 481
  • Why do you need locking? Also, a static class can't be a singleton. A singleton is, by definition, a class with a single instance. Static classes, by definition, have no instances. – Pete Mar 28 '13 at 13:03
  • 2
    Why do you need locking if you are only reading the values? And although it's not what you're asking for: please avoid singletons! They make your code harder to read and maintain. See this post for some great advice: http://stackoverflow.com/questions/1300655/whats-alternative-to-singleton – Wouter de Kort Mar 28 '13 at 13:03
  • I though Singleton was the way to go ... This is for storing only global settings that I use in my application. So Static would do? To be honest, I am not sure the best way to go ... – Miguel Moura Mar 28 '13 at 14:14
  • @Shapper: Yes, just use static. Singleton is architecturally more complicated, so don't use it unless necessary. See also, [Difference between static class and singleton pattern](http://stackoverflow.com/a/519530/18192) – Brian Mar 28 '13 at 22:36
  • This [question](http://stackoverflow.com/q/10281044/158779) is relevant if you want to learn about double checked locking and what could go wrong if done incorrectly on weak memory models. – Brian Gideon Apr 05 '13 at 02:56

5 Answers5

10

I would like to have this class static, as singleton

To implement a singleton correctly in C#, see Jon Skeet's excellent summary of what does and does not work:

http://csharpindepth.com/Articles/General/Singleton.aspx

I think I should implement a double lock? No?

No. Double-checked locking is a low-lock technique and therefore insanely dangerous on weak memory model hardware. The moment you stray even the slightest from a "blessed" pattern you have abandoned all hope of the program behaving predictably.

The only circumstances under which I would use double-checked locking are when all of the following are true:

  • Is there is extensive empirical evidence that single-checked locking produces poor performance?
  • Let's suppose single-checked performance is unacceptable. Single-checked locking usually produces bad performance due to contention, so step one is eliminate the contention. Can you eliminate the contention and get acceptable performance? I would only use double-checked locking if it was impossible to remove the contention or if the performance problem was caused by the several nanoseconds it takes to obtain an uncontended lock. In the latter case: wow, that's a fast program that those nanoseconds are the slowest thing, and wow, you have pretty serious performance requirements if you're counting individual nanoseconds.
  • Let's suppose that single-checked performance is unacceptable and cannot be fixed. Is there another low-lock technique, like using Interlocked.CompareExchange or Lazy<T> that has acceptable performance? At least you know that CompareExchange and Lazy<T> were written by experts and enforce memory barriers appropriately on all hardware. Don't use double-checked locking if there is a better tool already implemented.
  • Let's suppose that none of these tools give acceptable performance. Does double-checked locking give acceptable performance? If not then don't use it.
Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
5

as I see, you only read the data. So you need not lock here IMO. Use static constructor to init your variables, like Report (made it also static).

evgenyl
  • 7,837
  • 2
  • 27
  • 32
  • +1 for giving the OP an appropriate recommendation. It's great that everyone is answering the OP's question about singleton implementation, but your advice (not to use a singleton) is more important. – Brian Mar 28 '13 at 22:32
1

if you like it static and as Singleton, try it this way:

public static class Settings {
  private static readonly object LockObject = new object();
  private static LoggerSetting LoggerInstance;

  public static LoggerSetting LoggerSettings {
    get {
      lock (LockObject) {
         if (LoggerInstance == null)
           LoggerInstance = new LoggerInstance(); 

         return LoggerInstance;
       }
     }
   }

   public class LoggerSetting {
     public String Levels {
       get { return ConfigurationManager.AppSettings["Logger.Levels"]; }
     } 

     public const String Report = "team@xyz.com";
   }
}

and use it by:

string x = Settings.LoggerSEttings.Report;
1

Take a look at Jon Skeet's article Implementing the Singleton Pattern in C#

Simplest and good enough option:

public sealed class Settings
{
    private static readonly Settings instance = new Settings();

    public LoggerSettings Logger { get; private set; }
    public MailerSettings Mailer { get; private set; }

    // Explicit static constructor to tell C# compiler
    // not to mark type as beforefieldinit
    static Settings()
    {
    }

    private Settings()
    {
       Logger = new LoggerSettings();
       Mailer = new MailerSettings();
    }

    public static Settings Instance { get { return instance;} }
}

public class LoggerSettings {

    public LoggerSettings()
    {
        Levels = ConfigurationManager.AppSettings["Logger.Levels"];
    }

    public String Levels { get; private set; }
    public const String Report = "team@xyz.com";

}

// Mailer settings would look similar

As you are only reading data from this instance you don't need any locking. The singleton instance is created before any other thread can access it so no need to lock there also.

Usage:

 Settings.Instance.Mailer.Contact
pero
  • 4,169
  • 26
  • 27
-1

if you'd still like to go ahead and have a singleton instance

    public class MySettings{
    private static Object lockObj = new Object();
    private MySettings() {} // private .ctor

    private static MySettings _instance;

    public static MySettings MySingleSettings{
     get{
      if(_instance == null){
      lock(lockObj){
        if(_instance == null)
          _instance = new MySettings();
        }
      }
       return _instance;
    }
}
Vignesh.N
  • 2,618
  • 2
  • 25
  • 33
  • Do not use double-checked locking unless you have a demonstrated performance problem with single-checked locking. Double checked locking is dangerous. – Eric Lippert Mar 28 '13 at 15:02