Using NSKeyedArchiver/Unarchiver in custom getter/setter methods

Question

I'm trying to use a NSKeyedArchiver/NSKeyedUnarchiver to store data for a class's property in a file in order to persist the state of the property between launches of the app.

I have the property declared in MyClass.h like so:

@property (nonatomic, weak) NSDictionary *myDictionary;

and I've created custom getter and setter methods in MyClass.m to make sure the NSDictionary is written to disk:

-(NSDictionary *)myDictionary {
    NSDictionary *dictionary;

    NSString *path = [self pathToDataStore];
    if (![[NSFileManager defaultManager] fileExistsAtPath:path]) {
        dictionary = [NSDictionary dictionary];
    } else {
        NSData *archivedData = [NSData dataWithContentsOfFile:path];
        dictionary = [NSKeyedUnarchiver unarchiveObjectWithData:archivedData];
    }

    return dictionary;
}

-(void)setMyDictionary:(NSDictionary *)dictionary {
    NSString *path = [self pathToDataStore];
    NSDictionary *rootObject = [NSDictionary dictionaryWithDictionary:dictionary];
    [NSKeyedArchiver archiveRootObject:rootObject toFile:path];
    self.myDictionary = dictionary;
}

This is causing an infinite loop of calls to [self setMyDictionary], so clearly I'm doing something wrong. But what is the problem?

Answer 1

instead of:

self.myDictionary = dictionary;

you should do:

@synthesize myDictionary;
...

-(void)setMyDictionary:(NSDictionary *)dictionary {

...
myDictionary = dictionary
}

Your problem occurs cause calling:

self.myDictionary = dictionary; 

is equal to

[self setMyDictionary:dictionary];

Answer 2

The answer from debris explains why the use of the self.myDictionary syntax in the setter results in infinite recursion (because that "dot" syntax simply calls the setter again) and points out that, instead, you should use the instance variables in accessor methods.

Having said that, a couple of additional observations.

1. You could tighten up the existing code. For example, why does the setter create a new dictionary for archiving? You could just archive the dictionary that was passed to the setter (as well as log a message if the archive failed for any reason).

   -(void)setMyDictionary:(NSDictionary *)dictionary {
       if (![NSKeyedArchiver archiveRootObject:dictionary toFile:[self pathToDataStore]])
           NSLog(@"%s: archiveRootObject failed", __FUNCTION__);
       _myDictionary = dictionary;
   }
   

2. Every time you reference the self.myDictionary getter, it's going to re-retrieve from the archive, even if it was already retrieved. That's inefficient and might result in some unintended consequences (e.g. if (self.myDictionary == self.myDictionary)... will fail). You might want to define your getter to only retrieve the dictionary from archive if you have no dictionary:

   - (NSDictionary *)myDictionary {
       if (!_myDictionary) {
           NSString *path = [self pathToDataStore];
           if ([[NSFileManager defaultManager] fileExistsAtPath:path]) {
               _myDictionary = [NSKeyedUnarchiver unarchiveObjectWithFile:path];
           }
       }
   
       return _myDictionary;
   }
   

3. Note, because you have defined both accessor methods, you may want to have a @synthesize line that explicitly defines the ivar that follows the typical underscore convention:

   @synthesize myDictionary = _myDictionary;
   

4. Also, the use of the weak memory qualifier is curious (who owns this object if not this class?). I'd suggest making it strong.