Objective-C Singleton implementation, am i doing it right?

Question

In my class Deck i have

static Deck *gInstance = NULL;


+(Deck *) instance {
    @synchronized(self) {
        if (gInstance == NULL)
            gInstance = [[self alloc] init];
    }

    return (gInstance);
}

and an init method that looks like

-(id) init {

    if (gInstance != NULL) {
        return self;
    }

    self = [super init];

    if (self) {
       // Lots of clever things
    }
    gInstance = self;
    return self;

}

My concern here is mainly whether init is implemented correctly. Please let me know if what i wrote looks right to you.

Or ... is there a way i can make init private and prevent people (myself included) from seeing it altogether?

What is your purpose behind trying to enforce a private initializer? — Joshua Weinberg, Nov 18 '11 at 03:45
I come from Java world, where private methods make it impossible for an instance of the class to be instantiated by mistake. — James Raitsev, Nov 18 '11 at 04:15
There is no such thing as a truly private method in Objective-C. — Joshua Weinberg, Nov 18 '11 at 04:17
possible duplicate of [What does your Objective-C singleton look like?](http://stackoverflow.com/questions/145154/what-does-your-objective-c-singleton-look-like) — jscs, Nov 18 '11 at 08:05

score 6 · Accepted Answer · answered Nov 18 '11 at 03:33

6

That is a kind of odd singleton implementation. My favorite implementation is to use some newer functionality present in GCD.

+ (MyObj*)sharedObject;
{
    static dispatch_once_t once;
    static MyObj *sharedObj;
    dispatch_once(&once, ^ { shared = [[MyObj alloc] init]; });
    return shared;
}

I would recommend only doing that, nothing more, nothing less. Enforcing a strict singleton will only end in pain and is generally pointless and an anti-pattern.

As far as if there is anything strictly 'wrong' with your implementation, I believe you'd want to return gInstance in the initializer when it isn't NULL, not self.

answered Nov 18 '11 at 03:33

Joshua Weinberg

28,598
2
97
90

You're absolutely right. Thank you. Good call on return. I never heard about GCD before, is there a reference you can recommend i review? – James Raitsev Nov 18 '11 at 03:39
1

http://developer.apple.com/library/mac/#documentation/Performance/Reference/GCD_libdispatch_Ref/Reference/reference.html is the official reference and as always Mike Ash has a great discussion of it http://www.mikeash.com/pyblog/friday-qa-2009-08-28-intro-to-grand-central-dispatch-part-i-basics-and-dispatch-queues.html – Joshua Weinberg Nov 18 '11 at 03:44

Evan · Answer 2 · 2011-12-06T16:21:07.617

In general your constructor shouldn't enforce singleness. Your instance method should (which it does). The reason for this is that you may want a singleton of the object as well as the ability to create other instances of the object.

As for the code you provided your init is not thread safe. It would be possible for two separate threads to assign to gInstance. The thread that sets it first would leak memory. And other subtle bugs could result. For example if the singleton was some sort of shared datastore the thread that won the race would effectively have its data lost with respect to the rest of the program.

The reason I mention thread safety is that I personally have encountered many bugs related to singletons being used in multithreaded apps in unsafe ways. So only have singleton creation be thread unsafe when you are absolutely sure it won't be an issue.

In terms of how to implement it in a thread safe way I have seen it one of two says:

// first way
+ (Deck *)instance {
    static dispatch_once_t onceToken;
    static Deck *_shared;
    dispatch_once(&onceToken, ^{
        _shared = [[Deck alloc] init];
    });
    return _shared;
}

// second way
+ (Deck *)instance {
    static Deck *_shared;
    @synchronized(self) {
        if (_shared == nil) {
            _shared = [[Deck alloc] init];
        }
    }
    return _shared;
}

// init method
- (id)init {
    if ((self = [super init])) {
        // init stuff
    }
    return self;
}

Using dispatch_once requires libdispatch which means a minimum OS of iOS 4.0 or OS X 10.6. Using the @synchronized(self) should work on OSes prior to that.

Objective-C Singleton implementation, am i doing it right?

2 Answers2

Linked