5

I've found that singleton pattern on the net. It seems to me it has many things that can be optimized.

-In sharedMySingleton method, no need to call a retain ? I'm not sure...
-If not, why is there a retain in allocWithZone ?
-what is the use of @synchronized. The NSAssert make think that the block can be called many times, so if yes, there should be some more code to release previous memory, or exit the block clearly without just NSAsserting, and if no, why is there this NSAssert ?
-the chain beetween sharedMySingleton and alloc seems strange. I'd wrote myself something like :

+(MySingleton*)sharedMySingleton
{
    @synchronized([MySingleton class])
    {
        if (_sharedMySingleton == nil) _sharedMySingleton = [[self alloc] init];
        return _sharedMySingleton;
    }

    return nil;
}

+(id)alloc
{
    @synchronized([MySingleton class])
    {
        return [super alloc];
    }

    return nil;
} 

Singleton pattern

#import "MySingleton.h"

@implementation MySingleton

// ##########################################################################################################
// ######################################## SINGLETON PART ##################################################
// ##########################################################################################################
static MySingleton* _sharedMySingleton = nil;

// =================================================================================================
+(MySingleton*)sharedMySingleton
// =================================================================================================
{
    @synchronized([MySingleton class])
    {
        if (_sharedMySingleton == nil) [[self alloc] init];
        return _sharedMySingleton;
    }

    return nil;
}

// =================================================================================================
+(id)alloc
// =================================================================================================
{
    @synchronized([MySingleton class])
    {
        NSAssert(_sharedMySingleton == nil, @"Attempted to allocate a second instance of a singleton.");
        _sharedMySingleton = [super alloc];
        return _sharedMySingleton;
    }

    return nil;
} 

+ (id)allocWithZone:(NSZone *)zone  { return [[self sharedMySingleton] retain]; }
- (id)copyWithZone:(NSZone *)zone   { return self; }
- (id)retain                        { return self; }
- (NSUInteger)retainCount           { return NSUIntegerMax;  /* denotes an object that cannot be released */}
- (oneway void)release              { /* do nothing */ }
- (id)autorelease                   { return self; }

// ##########################################################################################################
// ##########################################################################################################
// ##########################################################################################################

// =================================================================================================
-(id)init 
// =================================================================================================
{   
    if (!(self = [super init])) return nil;

    return self;
}

// =================================================================================================
-(void) dealloc
// =================================================================================================
{
    [super dealloc];
}

// =================================================================================================
-(void)test 
// =================================================================================================
{
    NSLog(@"Hello World!");
}

@end
Oliver
  • 23,072
  • 33
  • 138
  • 230

3 Answers3

17

You shouldn't use this pattern at all (it's for a very special case of Singleton that you almost never need, and even in that case you generally shouldn't use it).

There are many good patterns discussed at What should my Objective-C singleton look like?, but most of them are outdated since the release of GCD. In modern versions of Mac and iOS, you should use the following pattern, given by Colin Barrett in the linked question:

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

I only copy it here rather than marking the question duplicate because the old question's highest-rated answers are out-dated.

Community
  • 1
  • 1
Rob Napier
  • 286,113
  • 34
  • 456
  • 610
  • What about the other methods : retain, retaincount, release, copyWithZone, ... ? Why are you talking about "modern" versions ? What do you mean by modern ? Any idea of why Apple did not update its singleton snippet with this kind of code ? – Oliver Apr 10 '12 at 08:54
  • 1
    By modern, I mean since the introduction of GCD. You should not override these methods. The only sample code that shows overriding these is here: https://developer.apple.com/library/mac/#documentation/Cocoa/Conceptual/CocoaFundamentals/CocoaObjects/CocoaObjects.html As it notes, this is a *strict* implementation of singleton, which the text above explains is generally not needed. I've opened a doc case to improve this documentation since it's confused a lot of developers. Mike Ash has a good post on this: http://www.mikeash.com/pyblog/friday-qa-2009-10-02-care-and-feeding-of-singletons.html – Rob Napier Apr 10 '12 at 14:15
  • What is the equivalent of 10.6 in terms of iOS ? – Oliver Apr 11 '12 at 08:52
  • It's the same in 10.6 and iOS. – Rob Napier Apr 11 '12 at 12:52
  • I mean, in Mike's blog, I can read "If you can target 10.6+, Grand Central Dispatch provides an extremely fast one-time initialization API which is perfect for a singleton." Wich version would be mentioned here if talking about iOS ? – Oliver Apr 11 '12 at 12:58
  • dispatch_once() was added in iOS 4.0. – Rob Napier Apr 11 '12 at 14:21
  • Thank you. Why returning an id instead of a MyFoo class ? – Oliver Apr 11 '12 at 15:04
  • 1
    You can return MyFoo* here. A lot of snippets have 'id' there so it's a little easier to cut and paste, but MyFoo* is better. Edited to make it clearer. – Rob Napier Apr 11 '12 at 15:32
0

No need to call retain because there is an alloc. Call retain on top of that would cause a memory leak. There is retain on allocWithZone because it's a singleton, so we don't want to create two different instances of the class. Rather allocating a new instance we augment the retain count of the singleton instance. Why is that? Probably to prevent somebody not aware of the singleton type of the class. If he calls allocWithZone and then release the instance, everything still work fine, and he actually accessed the shared singleton instance.

@synchronized is used to prevent two calls from two different threads to enter in the if statement at the same time. So the code is thread-safe.

The NSSAssert is probably to make the app 'crash' if two instances of the singleton are ever created. It's 'just-to-be-sure' code, also called defensive programming.

Concerning the chain beetween sharedMySingleton and alloc, I think it's just fine.

grandouassou
  • 2,500
  • 3
  • 25
  • 60
0

In sharedMySingleton method, no need to call a retain?

alloc returns a new instance with a reference count of one, so no retain is necessary.

If not, why is there a retain in allocWithZone?

According the rules, when you call allocWithZone, you own the reference, therefore allocWithZone much increase the reference count for you. But this implementation of allocWithZone is returning the singleton instance that was already created and owned by someone else (the sharedMySingleton method). So the sharedMySingleton method creates the object with alloc, therefore it becomes an owner. And then you get the same instance through allocWithZone, therefore you become a second owner of the same instance. So the retain count must go up since there are two owners now. That is why allocWithZone needs to retain.

What is the use of @synchronized?

@synchronized allows code to be called simultaneously by multiple threads. If you will never call sharedMySingleton from more than one thread, then it is not necessary and you may omit it.

The NSAssert make think that the block can be called many times, so if yes, there should be some more code to release previous memory, or exit the block clearly without just NSAsserting, and if no, why is there this NSAssert?

Because the class is intended to be a singleton, alloc should only be called once. The NSAssert() terminates the program if alloc is called more than once. Since NSAssert() terminates the program, when alloc is called a second time, no memory management is necessary.

Jay
  • 9,314
  • 7
  • 33
  • 40