Question

I am writing some singleton class in my framework and I do not want other developer to call init and alloc on those singleton classes so I have override those methods but since I am using ARC, so my code for singleton class is:

+(MFSingletonClass*) sharedInstance
{
    static dispatch_once_t pred = 0;
    dispatch_once(&pred, ^{
        sharedInstance = [[self alloc] init];
    });
    return sharedInstance;
}

and following are alloc and init methods:

+ (id)alloc {
    static dispatch_once_t pred = 0;
    dispatch_once(&pred, ^{
        sharedInstance = [super alloc];
    });
    return sharedInstance;
}

- (id)init {

    __weak typeof(self) weakSelf = self;
    static dispatch_once_t pred = 0;
    dispatch_once(&pred, ^{
        static BOOL init = NO;
        if (!init) {
            init = YES;
            typeof(self) strongSelf = weakSelf;
            strongSelf = [super init];
        }
    });
    return weakSelf;
}

I wanted to know whether my implementation of alloc and init method is proper or not?

Thanks in advance!

Was it helpful?

Solution

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:

  1. dispatch_once; and
  2. 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

OTHER TIPS

With singletons, you have two choices: Write the very simple sharedInstance method and be done with it, or try to do all kinds of ingenious tricks to keep people from instantiating more objects. Trying to instantiate the singleton through any call other than sharedInstance is a programming error. You don't write complicated code to prevent programming errors. And you'll never get it right anyway :-)

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top