8

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?

Improve this question

5 Answers 5

Reset to default
8

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...)

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

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.
4

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

Improve this answer

Comments

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];
Improve this answer

3 Comments

I'd also like to add the reminder to release and set to nil *foodlocations in the dealloc...
Ah, so your first example would work because that alloc set the retain count to 2, and releasing it is back to 1, right?
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];
3

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];
Improve this answer

2 Comments

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.
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.
2

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.

Improve this answer

2 Comments

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;
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!)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.