CODEWITHSUNDEEP
iosobjective-cdesign-patternssingleton
tbag
tbag

Reputation: 1278

Singleton with parameter iOS

I need to implement a singleton class that takes in a parameter. The same object will be passed as a parameter every single time so the resultant singleton object will always be the same.

I am doing something like the code below. Does this look ok? Is there a better way of achieving what I want to achieve?

  - (id)sharedInstanceWithAccount:(UserAccount *)userAccount {
      if (!sharedInstance) {
        @synchronized(self) {
          sharedInstance = [[[self class] alloc] initWithAccount:userAccount];
        }
      }

      return sharedInstance;
    }

    - (id)initWithAccount:(UserAccount *)userAccount {
      self = [super init];
      if (self) {
        _userAccount = userAccount;
      }

      return self;
    }

    - (id)init {
      NSAssert(false,
               @"You cannot init this class directly. It needs UserAccountDataSource as a paramter");
      return nil;
    }

    + (id)alloc {
      @synchronized(self) {
        NSAssert(sharedInstance == nil, @"Attempted to allocated a second instance of the singleton");
        sharedInstance = [super alloc];
        return sharedInstance;
      }
      return nil;
    }

Upvotes: 3

Views: 2884

Answers (2)

Yuchen
Yuchen

Reputation: 33036

There are a number of problem in this design:

  1. As recommended by Apple, should dispatch_once instead of @synchronized(self) for singleton:

    static MyClass *sharedInstance = nil;
static dispatch_once_t onceToken = 0;
dispatch_once(&onceToken, ^{
     sharedInstance = [[MyClass alloc] init];
     // Do any other initialisation stuff here
});
return sharedInstance;

    Refer to this question for more detail: Why does Apple recommend to use dispatch_once for implementing the singleton pattern under ARC?

  2. Bad API design to put singleton in alloc.

    As indicated by the name of the method alloc, it means that some memory will be allocated. However, in your case, it is not. This attempt to overwrite the alloc will cause confusion to other programmers in your team.

  3. Bad idea to use NSAssert in your -init.

    If you want to disable a method, disable it by putting this in your header file: 

    - (id)init __attribute__((unavailable));

    In this case, you will get a compile error instead of crashing the app at run time. Refer to this post for more detail: Approach to overriding a Core Data property: isDeleted

    Moreover, you can even add unavailable message: 

    - (id)init __attribute__((unavailable("You cannot init this class directly. It needs UserAccountDataSource as a parameter")));

  4. Sometime input parameters is ignored with no warning.

    In your following code, how would the programmer who is calling this function know that the input parameter userAccount is sometimes ignored if an instance of the class is already created by someone else? 

    - (id)sharedInstanceWithAccount:(UserAccount *)userAccount {
    if (!sharedInstance) {
        @synchronized(self) {
           sharedInstance = [[[self class] alloc] initWithAccount:userAccount];
        }
    }
    return sharedInstance;
}

In short, don't think it is a good idea to create singleton with parameter. Use conventional singleton design is much cleaner.

Upvotes: 2

Chope
Chope

Reputation: 79

objA = [Object sharedInstanceWithAccount:A];
objB = [Object sharedInstanceWithAccount:B];

B is ignored. userAccount in objB is A.

if userAccount B in objB, you will change sharedInstanceWithAccount.

- (id)sharedInstanceWithAccount:(UserAccount *)userAccount {
    static NSMutableDictionary *instanceByAccount = [[NSMutableDictionary alloc] init];
    id instance = instanceByAccount[userAccount];

    if (!instance) {
        @synchronized(self) {
            instance = [[[self class] alloc] initWithAccount:userAccount];
            instanceByAccount[userAccount] = instance;
        }
    }

    return instance;
}

Upvotes: 0

Related Questions