views:

89

answers:

2

I have numerous classes that use the various NSDictionary/NSArray collection classes as ivars but often I run into the problem of my collection class getting released before the containing class is released.

This seems to happen mostly with the collections classes and not with another model class (ie classes that I either created separately or other NS* non-collection classes).

Here are the two variations I've done and seen other people do: @implementation ClassX

// myDictionary declared as a property in the .h file as this:
// @property (nonatomic, retain) NSMutableDictionary *myDictionary;

@synthesize myDictionary;

- (id)int
{
    if (self = [super init])
    {
        // Option 1:
        // If I don't instantiate and assign with 'self',
        // myDictionary ivar will not be available 
        // at times in doSomething.

        myDictionary = [NSMutableDictionary dictionary];

        // Option 2:
        // Doing this, however will keep the dictionary around.
        // because I have invoked an extra retain on the dictionary
        self.myDictionary = [NSMutableDictionary dictionary];

        // Which one is more correct?   
    }
    return self;
}

- (void)doSomething
{
    // this will give the error about trying to invoke
    // a method on an already released instance
    [myDictionary objectForKey:@"myKey"];
}

- (void)dealloc
{
    // If I did self.myDictionary in 'init', I then
    // need to do this:
    [myDictionary release];
    [super dealloc];
}
@end

So which approach is the more correct way to hold an instance of NSDictionary within a class?

+2  A: 

Option 2 is correct; Option 1 is wrong.

But you left out the best option: myDictionary = [[NSMutableDictionary alloc] init].

Chuck
Why is option 1 wrong? The NSMutableDictionary class has a static initializer method that returns an empty dictionary.
Justin Galzic
@Justin because it's an autoreleased dictionary, which means it'll be gone by the next spin of the run loop, leaving your ivar with a stale pointer. The next time you try to access it after that, you'll crash.
Dave DeLong
`myDictionary = [[NSMutableDictionary dictionary] retain];` is good. Apple explained this really well in their [Memory Programming Guide](http://developer.apple.com/mac/library/documentation/cocoa/conceptual/MemoryMgmt/Articles/mmObjectOwnership.html)
Pumbaa80
@Pumbaa80: I don't see any benefit to writing `[[NSMutableDictionary dictionary] retain]` rather than using the standard alloc-init version I mentioned in my answer. The only difference AFAIK is that your version is less efficient (it's equivalent to `[[[[NSMutableDictionary alloc] init] autorelease] retain]`). Very good recommendation on the programming guide though.
Chuck
@Chuck: I totally agree. I just wanted to explain how version 1 could be "fixed", since I think Justin was confused about that.
Pumbaa80
+1  A: 

I recommend using

myDictionary = [[NSMutableDictionary alloc] init];

The memory is only within the scope of the method you're in if you call [NSMutableDictionary dictionary]. Once you leave the method, that memory goes with it which is why you need to alloc/init if you want to retain the values.

That's why you don't have to release if you don't encounter an alloc.

So for instance:

- (void) doSomething {

  // Do not need to release this string
  NSString *someText = @"Hello world!";

  // You need to release this string:
  NSString *otherText = [[NSString alloc] initWithString:@"Hello world!"];

  [otherText release];
}

Edited: Removed self after @mipadi @st3fan and caught my mistake. Forgot to post the change. Thanks for keeping me accountable.

Kennzo
If the `myDictionary` setter retains its argument, you're going to end up with a memory leak.
mipadi
Assuming `myDictionary` is a retaining property (why would it not be) ... this is bad advice and will leak memory.
St3fan
@mipadi @st3fan you're both wrong. he's setting the ivar directly, not using the setter method.
Dave DeLong
@Dave DeLong: Check the edit history, the answer *did* use a setter before.
mipadi
@mipadi aha, didn't notice that. thanks for keeping me vigilant! :)
Dave DeLong
Your explanation of how `[NSDictionary dictionary]` is memory-managed isn't really accurate. You can safely return an autoreleased dictionary from a method. It will be around at least until its autorelease pool is drained.
Chuck
Sorry for the confusion guys. Noobie SO user. Thanks for the input.
Kennzo
@Chuck: Isn't that only if it's explicitly mentioned as autorelease though? I understand returning an autoreleased object is safer but if it isn't doesn't mention autorelease, doesn't that memory then go away when it leaves the scope of the function? Correct me if I'm wrong. I'd genuinely like to know as I'm still learning this. Many thanks.
Kennzo
@Kennzo: You're right that we can't be sure it's autoreleased, but the memory management rules state that "[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." So the object is either autoreleased or may be treated like it is. In practice, constructors like that usually return an autoreleased object. There is no way with standard Objective-C objects to make the memory "go away when it leaves the scope of the function." Try to imagine how you'd write a method that returns such a thing.
Chuck
@Chuck: So therefore, once finishing the method the object was received in, if the object is not autoreleased, then the memory should be released afterwards, right? Or is it still treated as an autoreleased object and released when the pool is drained? Thanks again for your continued discussion.
Kennzo
@Kennzo: You don't need to know whether the object is autoreleased. That's not your concern. Your method just knows that it's received an object with the guarantees I listed in my other comment. And you definitely shouldn't release an object you don't own. I highly recommend Apple's memory management guide. It's pretty short but still manages to explain Cocoa memory management exhaustively: http://developer.apple.com/mac/library/documentation/cocoa/conceptual/MemoryMgmt/MemoryMgmt.html
Chuck