Retaining a locally-created NSMutableDictionary -- Necessary or not?

Question

Hey all, I've got a few question on a segment of code here that I'd really appreciate some feedback on. Basically I'm creating a temporary NSMutableDictionary in order to pull some values out of a plist. I'll then run a check on one of those values to determine if I want to do anything else with the plist.

Code is below, but my questions are as follows:

1) Main questions

Because I may be passing this locally-created Dictionary to another method, to be used there as source data, do I need to retain the dictionary here? If so, what is the proper method?

I'm still learning the language, and in addition to not knowing whether a retain/release is even necessary here, the method I've implemented below doesn't seem to do anything. When I dump the dictionary to the console before and after release, the same values are printed out (though perhaps that would happen even if the release were successful, since nothing else has overwritten that memory segment yet?)

2) Less important questions:

Does "return" immediately jump us out of the entire "multitaskingResumeCheck" function, without executing any further code inside? Or does it just jump out of the "else" loop? I tried putting the "NSMutableDictionary *dict" dictionary creation inside of the filemanager if loop and I was getting an error from the "NSNumber *wrappedATZ..." line about dict being undeclared. What was my error here?

Thanks in advance for all your help. I've put "//QUESTION TAG" on all pertinent lines.

- (void) multitaskingResumeCheck {

NSLog(@"Running multitaskingResumeCheck");

// Get the path to the "Documents" directory

NSArray *paths = NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES);
NSString *documentsDirectory = [paths objectAtIndex:0];

// Get the path to our plist ("Documents/foo.plist")

NSString *plistPath = [documentsDirectory stringByAppendingPathComponent:@"lastStopwatchState.plist"];

// check if our plist already exists in the Documents directory

NSMutableDictionary *dict; // QUESTION TAG (Why can't I do this in the "filemanager if" below)

NSFileManager *fileManager = [NSFileManager defaultManager];
if ([fileManager fileExistsAtPath:plistPath]) {
    // If it does, read it.     
    NSLog(@"dict existed, reading %@", plistPath);
    dict = [NSMutableDictionary dictionaryWithContentsOfFile:plistPath];

    [dict retain]; // QUESTION TAG (Necessary? If so, correct?)

} else {
    // If not, we're done here.
    return;
}

// Now check whether the timers were zeroed on exit

NSNumber *wrappedATZ = [dict valueForKey:@"allTimersZeroed"]; // QUESTION TAG (Got undeclared error if dict declaration was in wrong place (see above). Maybe I misunderstand "return"?
BOOL timersWereZeroed = [wrappedATZ boolValue];

// If they were not Zeroed, then we'll load in our data

if (!timersWereZeroed) {
    NSLog(@"Timers were not zeroed. Loading Data.");
    [self loadResumeData:dict];
}

// Dump the contents of the dictionary to the console
NSLog(@"dumping...");
for (id key in dict) {
    NSLog(@"key=%@, value=%@", key, [dict objectForKey:key]);
}   

[dict release]; // QUESTION TAG (Necessary? If so, correct?)

// Dump the contents of the dictionary to the console
NSLog(@"dumping...");
for (id key in dict) {
    NSLog(@"key=%@, value=%@", key, [dict objectForKey:key]);
}

}

JeremyP JeremyP · Accepted Answer · 2011-04-04T09:01:32

Your questions are more or less completely answered in the Cocoa Memory Management Rules although it may not be completely clear exactly how.

Firstly, you should refactor your code so that the brace structure matches the actual control flow. Namely, everything after the

else
{
    return;
}

Should be inside the if part so it look something like this:

- (void) multitaskingResumeCheck {

    // Some stuff not relevant to the question

    NSFileManager *fileManager = [NSFileManager defaultManager];
    if ([fileManager fileExistsAtPath:plistPath]) {
        // If it does, read it.     
        NSLog(@"dict existed, reading %@", plistPath);
        NSMutableDictionary* dict = [NSMutableDictionary dictionaryWithContentsOfFile:plistPath];

        [dict retain]; // QUESTION TAG (Necessary? If so, correct?)

        // Some stuff not relevant to the question

        if (!timersWereZeroed) {
            NSLog(@"Timers were not zeroed. Loading Data.");
            [self loadResumeData:dict];
        }

        // Dump the contents of the dictionary to the console
        NSLog(@"dumping...");
        for (id key in dict) {
            NSLog(@"key=%@, value=%@", key, [dict objectForKey:key]);
        }

        [dict release]; // QUESTION TAG (Necessary? If so, correct?)
    }

I've moved everything after the else part into the if part because it is effectively exactly the same. Since dict is now no longer used except in the if part, I've moved its declaration there. I've also deleted the second dump because dict no longer exists in the same scope. Other than that, this is functionally identical to what you had.

One of the points of the memory management rules says:

A received object is normally guaranteed to remain valid within the method it was received in, and that method may also safely return the object to its invoker.

This answers your first question. The dictionary you received from +dictionaryWithContentsOfFile: will remain valid within the method. That includes any methods that this method calls, passing dict as a parameter. So, in this instance, the retain and release are not necessary. The reason the second dump worked before is because the retain and release pair had no net effect on the ownership of the dictionary.

In answer to your second question, the return exits the function immediately without executing any code after it. This is why I was able to refactor the function in the way I did.

The reason you had an error when you moved the declaration of dict was that, in your code, dict was being referred to outside of the block (statements enclosed in { ... } are called a block) in which it was declared. C scoping rules prevent this.

As a final aside, to log the contents of a dictionary, you can do:

 NSLog(@"%@", dictionary);

No need to loop through the keys.

Retaining a locally-created NSMutableDictionary -- Necessary or not?

3 Answers