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

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 m开发者_如何学Cemory 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]);
}   

}

---

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.

---

Check the below code with the necessary changes & answers of your comments.

- (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 = nil; // Assign the initial value as nil if you're going to assign the actual value later in a conditional block.

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

    // It's not required to retain as it's being assigned using a class method which in-turn returns an auto-release object.

}

    if (dict) {  // Put a conditional check here whether dict is nil or not.

// Now check whether the timers were zeroed on exit

NSNumber *wrappedATZ = [dict valueForKey:@"allTimersZeroed"]; // No error should come here.
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]; No need to release an auto-release object.

    /** You were executing the below code after releasing the dict object. It might give garbage values which crashes the app. **/

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

---

1 . You should only retain an object in the place that will you use it. On the same note, each time you retain something you have to be sure to release it which should be handled in the same scope. You seem to do this properly in your code except that you release the dict and then use it in code afterwards. You might not need the retain because it will stay around until you leave the function but if you do retain it, it must be released.

If you pass a retained object to somewhere else you should relinquish responsibility by calling [object autorelease]. [NSMutableDictionary dictionaryWithContentsOfFile:plistPath]; does a similar thing when it returns an autoreleased copy of your dictionary.

2 . return; tells the code to leave the function entirely. The reason you couldn't declare dict in the if block is because it would only exist in that scope. As soon as you left the if code it would no longer exist.