Why am I having trouble with a deep copy in Objective C?

Question

I'm assuming my understanding of how to perform a deep copy isn't just there yet. The same with some sub-optimal memory handling that I'm performing down below. This code below probably depicts a shallow copy, and I believe that's where my problem might be. I have some cookie-cutter code for an example that looks like the following:

NSArray *user = [[xmlParser createArrayWithDictionaries:dataAsXML
                                              withXPath:kUserXPath] retain];
if([user count] > 0) {
    self.name = [[user valueForKey:@"name"] copy];
}

// Crash happens if I leave the next line un-commented.
// But then we have a memory leak.
[user release]; 

[xmlParser release];

Unfortunately when I comment out [user release], the code works, but we have an obvious memory leak. The method createArrayWithDictionaries:withXPath: was refactored last night when the SO community helped me understand better memory management. Here's what it looks like:

- (NSArray *)createArrayWithDictionaries:(NSString *)xmlDocument 
                               withXPath:(NSString *)XPathStr {
    NSError *theError = nil;
    NSMutableArray *dictionaries = [NSMutableArray array];
    CXMLDocument *theXMLDocument = [CXMLDocument alloc];
    theXMLDocument = [theXMLDocument initWithXMLString:xmlDocument
                                               options:0
                                                 error:&theError]; 
    NSArray *nodes = [theXMLDocument nodesForXPath:XPathStr error:&theError];

    for (CXMLElement *xmlElement in nodes) {
        NSArray *attributes = [xmlElement attributes];
        NSMutableDictionary *attributeDictionary;
        attributeDictionary = [NSMutableDictionary dictionary];
        for (CXMLNode *attribute in attributes) {
            [attributeDictionary setObject:[attribute stringValue]
                                    forKey:[attribute name]];
        }

        [dictionaries addObject:attributeDictionary];
    }

    [theXMLDocument release];
    return dictionaries;
} 

I'm guessing there's a couple of issues that might be going on here:

• Auto release on my dictionaries array is happening, thus my app crashing.
• I'm not performing a deep copy, only a shallow copy. Thus when the user array is released, self.name is done for.

With NSZombieEnabled, I see the following:

*** -[CFString respondsToSelector:]:
     message sent to deallocated instance 0x1ae9a0

Also, the final call where the backtrace shows this is crashing contains the following code in a separate module from the other two methods:

User *u = self.user;
NSString *uri = [NSString stringWithFormat:@"%@/user/%@/%@",
                         [self groupName], u.userId, kLocationsUri];

Between all the auto releasing/copies/retain happening between the client code and createArrayWithDictionaries:withXPath, I'm a bit confused as to the real problem here. Thanks again for helping me understand.

Answer 1

OK, you don't need to retain the return value from createArrayWithDictionaries: since you're not keeping it around. The return value is autoreleased. I'd strongly recommend reading up on how autoreleasing works. You only retain things that you intend to keep around in your object.

Also, user is an NSArray. If you call [user valueForKey:@"name"], you'll get another NSArray of values representing the values of the name key for each of the objects in users. Furthermore, how is the name property on your object defined? If you declared it as copy or retain (I believe retain is the default if you don't specify it yourself), you don't need to copy or retain the value. Indeed, the accessor should always be responsible for doing the memory management, not the caller. If you wrote your own accessor (i.e. you didn't use the @synthesize keyword), you need to make sure you do the memory management there.

I'm guessing what you meant to write was something more like this:

    NSArray *user = [xmlParser createArrayWithDictionaries:dataAsXML withXPath:kUserXPath];
    if ([user count] > 0)
        self.name = [[user objectAtIndex:0] objectForKey:@"name"];

    [xmlParser release];

I think your troubles are stemming from a misunderstanding of how memory management works in Objective-C.

Hope this helps.

Answer 2

Auto release on my dictionaries array is happening, thus my app crashing.

If the caller intends to keep the array around somewhere, it needs to retain it. Otherwise, it will crash when it tries to access the (now-deceased) object.

If the caller is going to store it in a property, it must use the self.dictionaries = […] syntax, not dictionaries = […]. The former is a property access, which calls the setter method; the latter is a direct instance variable assignment.

Coming back to your actual question, that of a deep copy: You need to get the sub-elements of every element and put them in each element's dictionary.

Basically, you need a recursive method (or a queue, but that's harder—file under premature optimization until you've proven you need it) that takes an element and returns a dictionary, and then you need to call this method on each of your element's child elements, and collect the results into an array and put that into the dictionary you're creating.

I would recommend making this recursive method an instance method of the element. Something like:

- (NSDictionary *) dictionaryRepresentation {
    NSMutableDictionary *attributeDictionary = [NSMutableDictionary dictionary];
    for (CXMLNode *attribute in attributes) {
        [attributeDictionary setObject:[attribute stringValue] forKey:[attribute name]];
    }

    NSArray *childElements = [self childElements];

    return [NSDictionary dictionaryWithObjectsAndKeys:
        attributeDictionary, @"attributes",
        [childElements valueForKey:@"dictionaryRepresentation"], @"childElements",
        nil];
}

Then you replace the loop in createArrayWithDictionaries:withXPath: with a similar valueForKey: message. I'll leave you to fill it in.

valueForKey: is Key-Value Coding's principal method. In both places, we're making use of NSArray's handy implementation of it.

(If the use of valueForKey: still doesn't make sense to you, you should read the KVC Programming Guide. KVC is vitally important in modern Cocoa, so you do need to read this sooner or later.)