11

I would like to take the GCD approach of using shared instances to the next step so I created the following code:

@implementation MyClass

static id sharedInstance;

#pragma mark Initialization

+ (instancetype)sharedInstance {
    static dispatch_once_t once;
    dispatch_once(&once, ^{
        sharedInstance = [[self alloc] init];
    });
    return sharedInstance;
}

- (instancetype)init {
    if (sharedInstance) {
        return sharedInstance;
    }
    @synchronized(self) {
        self = [super init];
        if (self) {
            sharedInstance = self;
        }
        return self;
    }
}

@end

I assume the sharedInstance method seems to be ok but I am unsure about the init method. The reason for creating this is that I don't want people using my SDK, to use the init method, and if they do ... make it bullet proof.

Ondrej Rafaj
  • 4,342
  • 8
  • 42
  • 65

3 Answers3

12

Instead of transparently redirecting calls to init to the singleton implementation which can cause very confusing behaviour for the users of your SDK, I suggest not allowing to call init at all:

+ (instancetype)sharedInstance {
    static dispatch_once_t once;
    dispatch_once(&once, ^{
        sharedInstance = [[self alloc] initPrivate];
    });
    return sharedInstance;
}

- (instancetype)init {
    @throw [NSException exceptionWithName:NSInternalInconsistencyException reason:@"..." userInfo:nil];
}

- (instancetype)initPrivate {
    if (self = [super init]) {
        ...
    }
    return self;
}
Leo
  • 37,640
  • 8
  • 75
  • 100
  • 1
    (self = [super init]) throws a warning and init without return nil will stop xcode from compiling ... it was in my edit – Ondrej Rafaj Apr 20 '15 at 14:17
  • 1
    @Ondrej the missing return can easily be "fixed" by using `@throw` instead of `[NSException raise:]`. And the `(self = [super init])` pattern doesn't create a warning, you must have something else messing it up. – Leo Apr 20 '15 at 14:47
  • 1
    Why not use `+initialize` to initialize the singleton instance? – Hyperbole Apr 20 '15 at 14:51
  • @Hyperbole there is no reason not to, if you prefer that option. So why not use the `dispatch_once` pattern ;) ? – Leo Apr 20 '15 at 16:07
  • @Hyperbole: Because using dispatch_once is an established pattern for all kinds of things; dispatch_once is used exactly where the functionality is needed, whereas +initialize code is written in a totally different place from where it is needed. Also with dispatch_once it is possible to call class methods that configure the singleton object before creating it. And dispatch_once can be used in C++ or C code, so you need to learn only once and use only one method. – gnasher729 Apr 20 '15 at 16:21
  • @Leo I asked because I've yet to get a cogent response. I use `+initialize` because it's called once per instance, is inherently thread-safe and requires no outside API knowledge. I don't have a particular argument against `dispatch_once`, just a wondering. Thanks, btw. – Hyperbole Apr 20 '15 at 17:51
  • @gnasher729 I see your points, though I'd suggest that you're combining unrelated functionality by initializing and returning in the same function scope. Why initialize and return in the same place? There's already a location for initialization that's thread safe and guaranteed to run once by the runtime without polluting the namespace with a new static variable and adding more code to a method that declares it only returns a shared instance. – Hyperbole Apr 20 '15 at 17:54
6

I would like to suggest new ways of solving your problem.

You can use NS_UNAVAILABLE in the header file just like this:

//Header file
@interface MyClass : NSObject
+ (instancetype)sharedInstance
- (instancetype)init NS_UNAVAILABLE;
//...
@end

In this case init function will not be available from outside, will not be suggested for autocompletion, and you'll be able to normally use the init method inside implementation file.

As you are making a singleton class I would suggest you to make new method unavailable too by adding this line to the header file:

+ (instancetype)new NS_UNAVAILABLE;

There is also an old way of making methods unavailable (which can be used in header too):

- (instancetype) init __attribute__((unavailable("Use 'sharedInstance' instead of 'init' as this class is singleton.")));

This can be used if you want to prompt some message about unavailability.

Peter O.
  • 32,158
  • 14
  • 82
  • 96
Just Shadow
  • 10,860
  • 6
  • 57
  • 75
  • 1
    This is not true with Xcode 7.3: no matter NS_UNAVAILABLE or the old way, Xcode will prompt compiler error saying the `sharedInstance` implementation can't use `init` as well. – Wingzero Jun 30 '16 at 05:59
0

The general opinion is that trying to protect your singleton against that kind of bug is pointless. Whoever calls [[LUIMain alloc] init] and creates a singleton gets what they deserved.

And the code that you wrote isn't thread safe anyway. If I call [[LUIMain alloc] init] while someone else calls sharedInstance, sharedInstance will return a different object than on the next call. (@synchronized (self) in the init method is pointless, because a second caller will have a different self).

Hima
  • 1,249
  • 1
  • 14
  • 18
gnasher729
  • 51,477
  • 5
  • 75
  • 98