CODEWITHSUNDEEP
objective-cios
Jason McCreary
Jason McCreary

Reputation: 72981

Objective-C calling instance method within Singleton

I created a Singleton class named DataManager. I've set up some caching. However, when attempting to call the cache instance methods from within another instance method of DataManager, I'm getting EXC_BAD_ACCESS error on the line:

NSMutableArray *stops = [[DataManager sharedInstance] getCacheForKey:@"stops"];

DataManager.h

@interface DataManager : NSObject {
    FMDatabase *db;
    NSMutableDictionary *cache;
}

+ (DataManager *)sharedInstance;
// ... more methods
- (id)getCacheForKey:(NSString *)key;
- (void)setCacheForKey:(NSString *)key withValue:(id)value;
- (void)clearCache;

DataManager.m

- (NSArray *)getStops:(NSInteger)archived {
    NSMutableArray *stops = [[DataManager sharedInstance] getCacheForKey:@"stops"];
    if (stops != nil) {
        NSLog(@"Stops: %@", stops);
        return stops;
    }

    stops = [NSMutableArray array];
    // set stops...

    [[DataManager sharedInstance] setCacheForKey:@"stops" withValue:stops];

    return stops;
}

It seems to occur when called from another view controller. That is the first view controller no error, second view controller, error.

This is my first attempt at a Singleton, so I'm sure I am making a simple mistake. But I am failing to see it myself.

Note: I've tried [self getCache...] with the same result.

UPDATE

Here is my Singleton implementation. Adapted from http://www.galloway.me.uk/tutorials/singleton-classes/

+ (DataManager *)sharedInstance {
    @synchronized(self) {
        if (!instance) {
            instance = [[super allocWithZone:NULL] init];
        }
    }

    return instance;
}

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

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

- (id)retain {
    return self;
}

- (unsigned)retainCount {
    return UINT_MAX;
}

- (oneway void)release {
    // never release
}

- (id)autorelease {
    return self;
}

- (id)init {
    if (self = [super init]) {        
        if (db == nil){
            BourbonAppDelegate *appDelegate = (BourbonAppDelegate *)[[UIApplication sharedApplication] delegate];
            [appDelegate createEditableCopyOfDatabaseIfNeeded];
            db = [[FMDatabase alloc] initWithPath:[appDelegate getDBPath]];
        }

        if (![db open]) {
            NSAssert(0, @"Failed to open database.");
            [db release];

            return nil;
        }

        [db setTraceExecution:YES];        
        [db setLogsErrors:TRUE];

        cache = [NSMutableDictionary dictionary];
        NSLog(@"cache: %@", cache);
    }

    return self;
}

Upvotes: 1

Views: 2371

Answers (2)

unexpectedvalue
unexpectedvalue

Reputation: 6139

Not a direct answer to your question, which was already answered.

However I'd just like to point out that your singleton implementation is sub-optimal. @synchronized is very expensive, and you can avoid using it every time you access the singleton:

if (!instance) {
    @synchronized(self) {
        if (!instance) {
            instance = [[super allocWithZone:NULL] init];
        }
    }
}

An even better way to initialize a singleton would be:

+ (DataManager *)sharedInstance {
    static DataManager *instance;

    static dispatch_once_t donce;
    dispatch_once(&donce, ^{
        instance = [[self alloc] init];
    });

    return instance;
}

Upvotes: 4

omz
omz

Reputation: 53551

Your cache object is autoreleased, so it's no longer in memory when you try to access it.

Use [NSMutableDictionary alloc] init] instead of [NSMutableDictionary dictionary] to get an instance that is retained.

Upvotes: 5

Related Questions