3

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;
    }
tbag
  • 1,268
  • 2
  • 16
  • 34
  • 2
    This is probably another indication as to why singletons are a bad idea. You would probably be better of using dependency injection in this case – Paulw11 Feb 21 '15 at 06:28
  • But how would I pass in the userAccount object even if I were to use dependency injection? – tbag Feb 22 '15 at 01:50
  • You would call `initWithAccount` when you instantiate `MyClass` and then pass this object to all other classes that need your `MyClass` instance – Paulw11 Feb 22 '15 at 02:13
  • I'm sorry maybe i'm not understanding this correct. Isn't this the same thing happening in the below function - (id)sharedInstanceWithAccount:(UserAccount *)userAccount { if (!sharedInstance) { @synchronized(self) { sharedInstance = [[[self class] alloc] initWithAccount:userAccount]; } } return sharedInstance; } – tbag Feb 22 '15 at 02:43
  • The problem with a singleton with a parameter is that there is the potential for two different calls to `sharedInstanceWithAccount` to pass different accounts (I know you say you won't but it is still possible). You can either use a factory pattern as per the answer with a dictionary, or just forget about singletons, instantiate the single class instance and pass it to every class that needs it using dependency injection – Paulw11 Feb 22 '15 at 02:53

2 Answers2

2

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.

Community
  • 1
  • 1
Yuchen
  • 30,852
  • 26
  • 164
  • 234
  • So how can I achieve what I am trying then? I want just one copy of this class to exist in the app and it depends on UserAccount. – tbag Feb 22 '15 at 01:49
  • Yes, if you use singleton, you will have only one copy of the class. If you want it to depends on UserAccount, just modify that value after you acquired the singleton. – Yuchen Feb 22 '15 at 01:51
  • Thats true. I mean how can I pass the UserAccount object to the class without taking in a parameter when constructing the class? I can make it a property and set it later but there's again a problem if someone forgets to set the property. – tbag Feb 22 '15 at 01:53
  • Okay, could you take a look at my edits to the question? Maybe that's what you are looking for. – Yuchen Feb 22 '15 at 01:59
0
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;
}
Chope
  • 79
  • 3