Objective C: Proper way to init an NSArray that is a @property

Question

I have a property in my class, which is an NSArray. I am retaining the property.

My question is, what is the proper way to add objects to that array without leaking and making the retain count too high?

This is what I am using:

.h:

NSArray *foodLocations;

@property (nonatomic, retain) NSArray *foodLocations;

// I make sure to synthesize and release the property in my dealloc.

.m

- (void)viewDidLoad {
    [super viewDidLoad];

    NSArray *tempFood = [[NSArray alloc] initWithArray:[self returnOtherArray]];
    self.foodLocations = tempFood;
    [tempFood release];

}

Is this the correct way to do it?

Solution 1

Yes this is correct and my preferred way of doing it as it renders the code more readable.

You are essentially allocating a temporary array and then assigning it to your property with a retain attribute, so it is safe to dealloc it as your property now "owns" it. Just remember that you still need to release it in your dealloc method.

You could also initialise the array and assign it to the property in the view controllers init method, depending on whether you need the property to be available to you before the view actually loads (i.e. in case you want to read the value of the property before pushing the view controller etc...)

Solution 2

you will typically want to declare the property copy in this case.

in most cases, immutable collection accessors should be copy, not retain. a lot of people get this wrong, and end up writing a lot of copying manually and sharing objects which should not be shared, thinking they are doing themselves good by cutting a corner.

copying in this form (the collection) is shallow. the objects in the array are not copied, just the array's allocation.

a good implementation of an immutable collection can simply implement copy by retaining self. if the argument is mutable, you want a copy anyhow (in the majority of cases).

your program is then simplified to a declaration of:

// note: copy, not retain. honor this if you implement the accessors.
@property (nonatomic, copy) NSArray * foodLocations;

and then the setter:

self.foodLocations = [self returnOtherArray];

of course, you must still init, dealloc, and handle thread-safety appropriately.

good luck

Solution 3

That looks fine. You don't actually need the tempFood variable, you can just do:

self.foodLocations = [[NSArray alloc] initWithArray:[self returnOtherArray]];
[self.foodLocations release];

or:

self.foodLocations = [[[NSArray alloc] initWithArray:[self returnOtherArray]] autorelease];

Solution 4

Or:

@synthesize foodLocations=_foodLocations;

then in code

_foodLocations = [[NSArray alloc] initWithArray:someOtherArray];

This avoids the autorelease required by

self.foodLocations = [[[NSArray alloc] initWithArray:someOtherArray] autorelease];

Solution 5

Yes, that is correct. Also good to keep in mind is what @synthesize is, in effect, doing for you. A synthesized (& retained) setter is functionally equivalent to the following code:

- (void)setVar:(id)_var {
    [_var retain];
    [var release];
    var = _var;
    [var retain];
    [_var release];
}

So, basically, every time you call self.var = foo, it releases the previously stored value and retains the new one. You handle the reference counting in your code, and the setter handles its own.

View more solutions

Comments

[Michael Fredrickson]

I'd also like to add the reminder to release and set to nil *foodlocations in the dealloc...

[Rog]

By the way do not worry about the effect any of this has on retain counts, they are largely irrelevant to your memory management practices. Lots of good discussion on why around here - do a search if you're interested.

[Nic Hubbard]

Ah, so your first example would work because that alloc set the retain count to 2, and releasing it is back to 1, right?

[Dave DeLong]

while the first one will work, it's kind of bad practice. Also, with the second one, you could just do: self.foodLocations = [self returnOtherArray];

[NSGod]

Matt, your code, as currently posted, is potentially unsafe: if var is currently non-nil, and someone makes a call to -setVar:, passing in the existing var as the parameter, the first call of [var release] would cause var to be deallocated. The subsequent attempt to retain would cause a crash and/or unexpected results. It should be: [_var retain]; [var release]; var = _var;

[Matt J]

Thanks, NSGod. I missed that. I've updated the code to accomodate. (and this is why it's best to just let @synthesize do it's thing!)

[Eric Reid]

This is a potential (actually, very likely) memory leak. If that code is in your viewDidLoad method, which has a high potential of being called multiple times, the first alloced object will not be released when you set your ivar to the second alloced object.

[user3443701]

That's why viewDidUnload should contain self.foodlocations = nil. If it's something you create in viewDidLoad, you should probably dump it in viewDidUnload. Letting it hang around when your view isn't loaded uses unnecessary memory.