views:

945

answers:

7

From the perspective of an API end-user who needs to "get an the instance" of a Singleton class, do you prefer to "get" an the .Instance property or "call" a .GetInstance() Method?

public class Bar
{
    private Bar() { }

    // Do you prefer a Property?
    public static Bar Instance
    {
        get
        {
            return new Bar();
        }
    }

    // or, a Method?
    public static Bar GetInstance()
    {
        return new Bar();
    }
}

I'm just looking for some developer/community feedback.

+6  A: 

In C#, I would far prefer .Instance, as it fits in with the general guidelines.

Noon Silk
+1 agreed.Passing parameters shouldn't be necessary either.
Mickel
@Mickel dependency injection coupled with a singleton pattern?
Rex M
Personally, I've never even written a `Singleton` for my own legitimate, and I'm generally dubious of it's need.
Noon Silk
@silky What do you do in the case of a WinForm that should really only be created/accessible as a single instance across multiple DLLs?
Michael Todd
@silky they are used far more than they should be, to be sure - but any time we have to deal with the fact that available resources are finite, we can't avoid globals of some kind. A singleton is just one formalized way of enforcing a global.
Rex M
Michael Todd: I've never written a 'real' winforms app :) Just speaking from my own experience.
Noon Silk
Rex: Indeed, I don't doubt the idea in general, just in practice, I don't find it comes up that often.
Noon Silk
I agree with this with one additional condition: due to the implied purity requirement of property getters, the `Instance` property *should NOT* create the instance if one does not currently exist. For this case, you need to create it with a field initializer (`static FieldType field = new FieldType()`) or inside of a static constructor.
280Z28
+2  A: 

Depends. Do you need to pass parameters? If so, I'd do GetInstance(). If not, probably doesn't matter (at least from a calling standpoint, since they're really both methods anyway; however, it does matter if you're trying to be more standards-based and, in that case, an instance appears to be better).

Michael Todd
A: 

I prefer property, these are standard patterns.

Jonathan Shepherd
+4  A: 

As with just about everything, it depends :)

If the singleton is lazy-loaded and represents more than a trivial amount of work to instantiate, then GetInstance() is more appropriate, as a method invocation indicates work is being done.

If we're simply masking to protect the singleton instance, a property is preferable.

Rex M
But specifically for a singleton, only the first invocation should do the work. The following invocations should just return the previously prepared instance.
Hans Kesting
@Hans of course, but in a lazy-loaded singleton the caller doesn't know whether it has been instantiated, so every call is potentially the heavy one.
Rex M
+9  A: 

If you want to create singleton you cannot just return new object on every GetInstance call or Instance property getter. You should do something like this:

public sealed class Bar
{
    private Bar() { }

    // this will be initialized only once
    private static Bar instance = new Bar();

    // Do you prefer a Property?
    public static Bar Instance
    {
        get
        {
            return instance;
        }
    }

    // or, a Method?
    public static Bar GetInstance()
    {
        return instance;
    }
}

And it doesn't really matter which way of doing that you choose. If you prefer working with properties choose it, if you prefer methods it will be ok as well.

RaYell
Are you sure? I thought the same thing at first, but the private constructor ensures that only that class can create instances of itself.In any case, you can do away with the private constructor in yours.
Ty
All valid points; it should probably also be `sealed`.
Marc Gravell
@RaYell I think his code sample is just to demonstrate the difference between the property and the method.
Rex M
@Ty - if you do away with the private ctor, then there is an implicit *public* ctor - not what we want. And yes: for a singleton you absolutely should re-use `instance` (or similar).
Marc Gravell
Private constructor allows the instances to be created only by the class members. And that's what happens in the code. It will work, but it won't be a singleton. You will have a new instance created every time to use `Instance` property or `GetInstance()` method.
RaYell
@Rex M - I'm aware of that and I have given my answer above. I just thought it is a good idea to point out a flaw in the example code because it's written that this should be singleton.
RaYell
@Marc Gravell You're right, but I'd argue that the singleton shouldn't be holding references to itself. There should be a BarSingleton passing out the same instance of Bar. IMO.
Ty
And non-serialisable.
Noon Silk
@Ty - presumably a nested type? Yes, that works too, and helps keep the `beforefieldinit` stuff tidy.
Marc Gravell
@Ty - maybe this will help you understand, http://msdn.microsoft.com/en-us/library/ms998558.aspx
RaYell
A: 

As @Rex said, it depends on the semantic you want to convey.

GetInstance() does not necessarily imply a singleton instance. So, I would use GetInstance() in the case where the instance creation happens on demand, direct new is not desireable and the instance could be, but is not guaranteed to be the same. Object pools fit these criteria as well. (In fact, a singleton is a specialization of an object pool with state preservation :-))

Static Instance property on the other hand implies a singleton and preserved instance identity.

Btw, as @RaYell mentioned your sample code is not a singleton, so you shouldn't be using Instance property. You can still use the GetInstance() method in this case, as it would serve as an instance factory.

Franci Penov
+3  A: 
public class Singleton
{

    private volatile static Singleton uniqueInstance;
    private static readonly object padlock = new object();

    private Singleton() { }

    public static Singleton getInstance()
    {
        if (uniqueInstance == null)
        {
            lock (padlock)
            {
                if (uniqueInstance == null)
                {
                    uniqueInstance = new Singleton();
                }
            }
        }
        return uniqueInstance;
    }
}

In the above code double checking is implemented ,first it is checked if an instance is is created and if not lock has been established .Once in this block

                if (uniqueInstance == null)
                {
                    uniqueInstance = new Singleton();
                }

if the instance is null then create it.

Also, the uniqueInstance variable is declared to be volatile to ensure that assignment to the instance variable completes before the instance variable can be accessed.

Raghav Khunger
Nice double check lock. Though if you only need to instantiate your singleton, use:private static readonly Singleton uniqueInstance = new Singleton();
bryanbcook