Is implementing a singleton using an auto-property a good idea?

Question

I recently found out about auto-properties and like them quite a lot. At this moment I am trying to use them everywhere where I can. Not really to just be able to use them everywhere, but more to see how well they work in most situations.

Now I am making a singleton and thought:"Hey, let's try auto-properties here as well".

public class MySingleton
{
    public static MySingleton MySingleton { get; private set; }

    private MySingleton() {}

    static MySingleton() { MySingleton = new MySingleton(); }
}

So my question is: "Is it a good idea to implement a singleton like this?"

I am not asking whether a singleton in general is a good idea.

Answer 1

I wouldn't personally do that. I don't like using automatically implemented properties with a private setter that you never call where really you want a read-only property backed by a read-only variable. It's only one more line of code to be more explicit about what you mean:

public sealed class MySingleton
{
    private static readonly MySingleton mySingleton;
    public static MySingleton MySingleton { get { return mySingleton; } }

    private MySingleton() {}

    static MySingleton() { mySingleton = new MySingleton(); }
}

This way no-one's even tempted to change the singleton class to reassign either the property or the variable, because the compiler will stop them. They'd have to add a setter and/or make the variable non-readonly, which is a bigger change - one which hopefully they'd reconsider.

In other words:

• Yes, it will work.
• No, I don't think it's a good idea.

As of C# 6, this is easier with read-only automatically implemented properties though:

public sealed class MySingleton
{
    public static MySingleton MySingleton { get; } = new MySingleton();    
    private MySingleton() {}         
    static MySingleton() {}
}

Answer 2

I would approach this from a slightly different direction than Jon. Regardless of whether the object is a singleton, is it logically best modeled as a property in the first place?

Properties are supposed to represent... properties. (Captain Obvious strikes again!) You know. Color. Length. Height. Name. Parent. All stuff that you would logically consider to be a property of a thing.

I cannot conceive of a property of an object which is logically a singleton. Maybe you've come up with a scenario that I haven't thought of; I haven't had any Diet Dr. Pepper yet this morning. But I am suspicious that this is an abuse of the model semantics.

Can you describe what the singleton is, and why you believe it to be a property of something?

All that said, I myself use this pattern frequently; usually like this:

class ImmutableStack<T>
{
    private static readonly ImmutableStack<T> emptystack = whatever;
    public static ImmutableStack<T> Empty { get { return emptystack; } }
    ...

Is "Empty" logically a property of an immutable stack? No. Here the compelling benefit of being able to say

var stack = ImmutableStack<int>.Empty;

trumps my desire for properties to logically be properties. Saying

var stack = ImmutableStack<int>.GetEmpty();

just seems weird.

Whether it is better in this case to have a readonly field and a regular property, or a static ctor and an autoprop, seems like less of an interesting question than whether to make it a property in the first place. In a "purist" mood I would likely side with Jon and make it a readonly field. But I also frequently use the pattern of making autoprops with private setters for logically immutable objects, just out of laziness.

How's that for taking all sides of a question?

Answer 3

I don't see any reason why this wouldn't be correct. After all, auto-properties are just syntactic sugar for accessors for a private (compiler-generated) backing field.

Answer 4

Sure, I don't see any problem with that.

Answer 5

Auto property? No, I wouldn't but using a setter on a singleton, yes I did. I think you want more control over it than an auto property will give you.

... constructive feedback is more than welcome here, please. This feels like a brave (or foolish) move in this esteemed company ...

My scenario, a WPF app that has a Current Project than can be loaded and saved. The current project settings are used all over the application...

• in WPF bindings in the UI so the user can change the settings, hence the INotifyPropertyChanged interface.
• I also use Fody.PropertyChanged but that doesn't do static property changes, hence NotifyStaticPropertyChanged.
• INotifyPropertyChanged works fine with WPF bindings on the singleton's properties.
• An instance of the Settings is [de]serialized using JSON.NET, hence the [JsonIgnore] attribute on the singleton. I validate it before loading it into the singleton or saving it to disk.
• the logger is Serilog. You log stuff, right?
• the singleton is public with a public getter because WPF bindings only work on public properties. The internal setter doesn't affect it. All the properties of Settings are public for WPF binding.

I left all the "noise" in the code because some may find it useful.

class Settings : INotifyPropertyChanged
{
    private static Settings _currentSettings = new Settings();

    /// <summary> The one and only Current Settings that is the current Project. </summary>
    [JsonIgnore] // so it isn't serialized by JSON.NET
    public static Settings CurrentSettings //  http://csharpindepth.com/Articles/General/Singleton.aspx 
    {
        get { return _currentSettings; }

        // setter is to load new settings into the current settings (using JSON.NET). Hey, it works. Probably not thread-safe.
        internal set 
        {
            Log.Verbose("CurrentSettings was reset. Project Name: {projectName}", value.ProjectName);
            _currentSettings = value;
            _currentSettings.IsChanged = false;
            NotifyStaticPropertyChanged("CurrentSettings");
        }
    }

    // http://10rem.net/blog/2011/11/29/wpf-45-binding-and-change-notification-for-static-properties
    /// <summary> Fires when the Curent CurrentTvCadSettings is loaded with new settings
    ///           Event queue for all listeners interested in StaticPropertyChanged events. </summary>
    public static event EventHandler<PropertyChangedEventArgs> StaticPropertyChanged = delegate { };
    private static void NotifyStaticPropertyChanged(string propertyName)
    {
        StaticPropertyChanged?.Invoke(null, new PropertyChangedEventArgs(propertyName));
    }

    // various instance properties including ....

    public string ProjectName {get; set;}

    [JsonIgnore]
    public bool IsChanged { get; set; } 
}

usage to set the singleton to a newly loaded project, settings is simply

Settings settings = new Settings();
// load, save, deserialize, set properties, go nuts
Settings.CurrentSettings = settings;

The setter probably isn't thread-safe but I only set it in one place from the UI thread so I'm not scared. You could make it thread safe following the esteemed advice at http://csharpindepth.com/Articles/General/Singleton.aspx

I realise the OP didn't ask about WPF but I think it's relevant to show why you may want to set a singleton. I did it because it is the simplest solution.

Answer 6

Wrap up + alternative syntax with lambda style (>= C# 6) aka computed property (aka expression-bodied member):

The code is functionally fully equivalent to the answer of Jon Skeet, here again with "Instance". I don´t expect kudos for this, but I think this updated wrapup with explanation at one place is it worth, because this C#6 question and answers extend the old closed thread where variations of Singleton have been discussed.

You could argue the automatic-property-style with an explicitly missing set expresses clearer the intent of a read-only property, but in the end it is style based, and both styles are common in modern C#.

public sealed class MySingleton
{
    public static MySingleton Instance => new MySingleton(); // Assure that instantiation is only done once (because of static property) and in threadsafe way, and as this is an alternative style for a readonly-property, there is no setter
    private MySingleton() {} // Assure, that instantiation cannot be done from outside the class        
    static MySingleton() {} // Assure partly lazyness, see below     
}

Here is all detail and historical reference at one place:

Discussion about lazyness: http://csharpindepth.com/Articles/General/Beforefieldinit.aspx
Simplified summary: The above implementation behaves lazy as long as no other static fields/properties are introduced in the class. (But this is an area which can be .NET implementation-dependent.)

Discussion concerning different implementations of Singleton and thread safety (older C# styles): http://csharpindepth.com/Articles/General/Singleton.aspx

Original thread (closed): Singleton Pattern for C#