iOS Singleton Variables Not Keeping Their Values

Question

So I'm still kind of new to Objective-C, and this was my first app that I'm now updating. The idea is this: The whole app is basically various lists of stuff. It asks the API for 15 posts, shows those with a Load More button. Click Load More, it loads 15 more, etc. The API that it loads these from has a token system with a timeout built in. Too long between requests, and you have to get a new token. So I want to have a singleton to use anywhere in my app so I can just do [APIMachine getToken] and behind the scenes, it checks if the time since the last request was too long (or this is the first request), if so, gets a new token, otherwise returns the one we already have. I'm following the singleton pattern I've found in so many places, but every time the Load More button uses [APIMachine getToken]it gets either nothing or something completely random. I had it print this stuff in the logs, and one time I even got a UITableViewCell as my token. Looks like variables are being overwritten somehow. But I really can't figure it out.

So here it is:

static PoorAPI2 *_instance;
@implementation PoorAPI2

@synthesize apiToken, timeOpened, tokenTTL;

+ (PoorAPI2*)sharedAPI
{

    @synchronized(self) {
        if (_instance == nil) {         
            _instance = [[super allocWithZone:NULL] init];
        }
    }
    return _instance;
}

-(NSString *)API_open{

    //boring code to get api token redacted

if ([doneness isEqualToString:@"success"]) {
    NSDictionary *data = [json objectForKey:@"data"];
    apiToken = [data objectForKey:@"api_token"];
    tokenTTL = [data objectForKey:@"ttl"];
    timeOpened = [NSDate date];

}else{
    NSLog(@"FFFFFFFUUUUUUUUUUUU this error should be handled better.");
}

return apiToken;    
}

-(BOOL)isConnectionOpen{
    return ([timeOpened timeIntervalSinceNow]  > tokenTTL);
}

-(NSString *)getToken{
    if([self isConnectionOpen]){
        return apiToken;
    }else{
        return [_instance API_open];
    }
}

-(id)init{
    if(self = [super init]){
        apiToken = [[NSString alloc] initWithString:@""];
        timeOpened = [[NSDate alloc] initWithTimeIntervalSinceNow:0];
        tokenTTL = 0;
    }
    return self;
}

+ (id)allocWithZone:(NSZone *)zone
{   
    return [[self sharedAPI]retain];    
}

- (id)copyWithZone:(NSZone *)zone
{
    return self;    
}

- (id)retain
{   
    return self;    
}

- (unsigned)retainCount
{
    return NSUIntegerMax;  //denotes an object that cannot be released
}

- (void)release
{
    //do nothing
}

- (id)autorelease
{
    return self;    
}

@end

I can only hope I'm doing something seriously foolish and this will be a hilarious point-and-laugh-at-that-guy thread. Then at least my app will work.

Answer 1

In API_open, you store three objects in instance variables, but they're not objects you own, so they'll probably be gone by the time you need them and replaced by something unpredictable. You need to retain them or use proper setters.

Answer 2

You problem is:

static PoorAPI2 *_instance;

C, and by inheritance Objective-C, do not initialize variables. Just change to:

static PoorAPI2 *_instance = nil;

Also I am of the school that adding extra code to try to prevent the singleton from being used as a single is a total waste of time, and only give you more code with more possibilities for bugs.

So if I was you then I would remove every method from +[PoorApi2 allocWithZone:] and down. Objective-C is a dynamic language and if a client wanted to instantiate a second instance of your singleton then it would be able to do so despite all your wasted extra lines of code. At the most I would add a log like this:

-(id)init{
    if (_instance) NSLog(@"WARNING: PoorAPI2 already has a shared instance.");
    if(self = [super init]){
        apiToken = [[NSString alloc] initWithString:@""];
        timeOpened = [[NSDate alloc] initWithTimeIntervalSinceNow:0];
        tokenTTL = 0;
    }
    return self;
}

Creating a second instance of a singleton is a programming error and should be caught in development. Not a problem you should add extra lines of code to hide.