You ask whether your "implementation of alloc and init method is proper or not?"
Depends on how you define "proper"...
Your code is not incorrect as far as it goes, in that it usually does work, but the init is, let's say, over-cautious and a little incorrect, and there is a trick or two you missed.
Looking at the init you double up checking whether the initialisation has been run:
- dispatch_once; and
- a
BOOL flag init.
The block passed to dispatch_once will be executed at most once, and at that time init will be NO. Overkill. Removing that you get:
- (id)init
{
__weak typeof(self) weakSelf = self;
static dispatch_once_t pred = 0;
dispatch_once(&pred,
^{
typeof(self) strongSelf = weakSelf;
strongSelf = [super init];
});
return weakSelf;
}
Now consider [super init], this is a method call on the current instance, that is though it is not explicit this expression references self. Given that your efforts to avoid a retain cycle are wasted. Furthermore in those efforts you assigned the result of [super init] to a block-local variable strongSelf while init returns weakSelf - this is the reason for the "usually does work" remark above, most calls to [super init] do actually return the same object, but the need not. If we remove the use of weak references we solve both these issues at once:
- (id)init
{
static dispatch_once_t pred = 0;
dispatch_once(&pred,
^{
self = [super init];
});
return self;
}
Bot doesn't this leave us with a retain cycle? No, the object referenced by self does not keep a reference to the block, so the block having a reference back to the object doesn't matter.
The "as far as it goes" remark above
The default alloc method just calls allocWithZone:, the latter takes a memory zone - something which is no longer used but exists for historical reasons. Therefore you should really implement allocWithZone: rather than alloc to catch all allocations - the code would follow the same algorithm as your alloc.
Then there is the copy and copyWithZone: pair. If you want a singleton you don't copies of it, so you should implement copyWithZone: (which copy just calls as will alloc/allocWithZone:) and return self:
- (id) copyWithZone:(NSZone *)zone
{
return self;
}
The missed trick
So your code works as far as it goes, apart from the case where [super init] returns a different object, but was overcomplicated; clean up init to address that.
However if you dig up Apple's now deprecated Cocoa Objects documentation you'll find Apple's old recommendation for producing a true singleton. Under ARC you can ignore the MRC overrides, and you can add the use of GCD as you have done to ensure concurrent correctness, and that will leave you with one trick: Apple implement allocWithZone: by calling sharedManager (your sharedInstance) and having that in turn call allocWithZone on super. With your code that would be:
+ (MFSingletonClass*) sharedInstance
{
static MFSingletonClass *sharedInstance = nil;
static dispatch_once_t pred = 0;
dispatch_once(&pred,
^{
sharedInstance = [[super allocWithZone:NULL] init];
});
return sharedInstance;
}
+ (id)allocWithZone:(NSZone *)ignore
{
return [self sharedInstance];
}
Which is a little cleaner, but your approach works fine as well.
And kudos for knowing the difference between a true singleton and just a shared instance!
HTH
