C# Singleton design pattern basics

Question

Just recently started to try and learn some designs patterns. Currently trying to get my singleton to return a new object. However it keeps throwing the error "Cannot convert method group 'getInstance' to non-delegate type 'MainWdinow.CustomerLoader'. did you intend to invoke the method?

here is the code for the design pattern method

  public class CustomerLoader
    {
        private static Customer firstInstance = null;
        public static Customer getInstance()
        {
            if(firstInstance ==null)
            {
                firstInstance = new Customer();
            }

            return firstInstance;
        }
    }

Here is where I try to call the method and I get the error mentioned above

CustomerLoader t = CustomerLoader.getInstance();

I want my singleton to do the job of the code below and create a new instance of the customer object

Customer T = new Customer;

First look and I think you should call it like this `Train t = CustomerLoader.getInstance();` Also are you trying to create a Train or Customer? Any inheritance there? Maybe you want to have `private static Customer firstIntance = null;` and then you can do `Customer t = CustomerLoader.getInstance();` — , Dec 07 '18 at 10:47
Also look at `Lazy`... it's meant for what you're doing there. — sellotape, Dec 07 '18 at 10:53
@BART yep was meant to be customer throughout instead of train, not had a coffee yet, sorry for any confusion, made an edit and updated the code! thanks again — , Dec 07 '18 at 10:54

Wanton · Answer 1 · 2018-12-07T11:06:47.180

2

Use this. It's also thread safe unlike your version

private static readonly Lazy<Customer> _instance = new Lazy<Customer>(() => new Customer());
public static Customer Instance => _instance.Value;

But you should really use dependency injection instead singletons.

And your naming is off, it looks like Java, check this https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/names-of-type-members

private members are not covered by guidelines. But even Microsoft uses _camelCase for private fields in corefx https://github.com/dotnet/corefx/blob/master/Documentation/coding-guidelines/coding-style.md

edited Dec 07 '18 at 11:06

answered Dec 07 '18 at 10:56

Wanton

800
6
9

This is how I do my singleton classes, if anyone has a better way I would appreciate you posting it. I actually do `public static Session Instance { get { return lazy.Value; } }` to return the object, I presume `=>` does the same? – Dec 07 '18 at 10:58
BTW, thread-safe yes, but only if you use one of the other constructors that specify you want it to be. – sellotape Dec 07 '18 at 11:03
@sellotape what does that has to with property being thread safe? – Wanton Dec 07 '18 at 11:05
@Matt yes `=> lazy.Value;` is same as `{ get { return lazy.Value; } }` https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/statements-expressions-operators/expression-bodied-members – Wanton Dec 07 '18 at 11:09
@Wanton - `Lazy` has several constructors; not all instances are thread-safe. However, I see that the default - without specifying thread-safety - is actually thread-safe anyway (`LazyThreadSafetyMode.ExecutionAndPublication`), so I withdraw the original assertion. – sellotape Dec 07 '18 at 11:10
@Wanton Thanks! :) – Dec 07 '18 at 11:14
I'd recommend using `LazyWithNoExceptionCaching` vs the default `Lazy` - https://stackoverflow.com/a/42567351/34092 (if `new`ing up `Customer` might throw an exception). – mjwills Dec 07 '18 at 11:34
@mjwills that sounds like a bad idea. If it throws once you want it to throw every time like that default `new Lazy` does and not retry it. If you have that kind of requirements using a singleton pattern like this is not a good idea. – Wanton Dec 07 '18 at 11:43
@Wanton That may be true, it depends on context. If there was a transient error relating to loading it from the database then it may be helpful for it to retry, for example (with `Lazy` won't do). _The reason I mention the issue is that the OP's **original** code won't have the exception-caching behaviour that your solution does - thus `LazyWithNoException` will act more like their existing code in terms of exception handling._ – mjwills Dec 07 '18 at 11:45
1

@mjwills true about OP's original code, that it will do retrying if there is an exception. – Wanton Dec 07 '18 at 11:55

score 0 · Answer 2 · answered Dec 07 '18 at 10:55

Use this example:

public class Singleton    
{
    private static Singleton instance = null;
    private static readonly object padlock = new object();

Singleton()
{
}

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

Singleton should be about one class, not three.
Be careful with it cause your implementation is not thread-safe. See this padlock variable here. It prevents creating multiple instances in a multi-treaded applicatoins.

score -1 · Answer 3 · answered Dec 07 '18 at 11:01

CustomerLoader.getInstance

Is a function, you cant assign it to CustomerLoader

Your code should look more like that:

class Train
{
    protected Train() { }

    protected static Train instance;

    public static Train GetSingeltonInstance()
    {
        if(instance == null)
        {
            instance = new Train();
        }

        return instance;
    }
}

class TainUser
{
    private readonly Train train;
    public TainUser()
    {
        train = Train.GetSingeltonInstance();
    }
}

C# Singleton design pattern basics

3 Answers3