Memory management technique for Objective-C iVars/properties

Question

Is the following code doing anything unnecessary?

@interface MyClass {
   NSArray   *myArray;
}

-(void)replaceArray:(NSArray *)newArray;

@implementation MyClass

-(void)replaceArray:(NSArray *)newArray {
   if( myArray )
   {
      [myArray release];
      myArray = nil;
   }

   myArray = [[NSArray alloc] initWithArray: newArray];
}

@end

What if I made the following changes:

1) Made myArray a property:

@property (nonatomic, retain) NSArray myArray;

2) Changed the assignment to:

self.myArray = [NSArray arrayWithArray: newArray];

Would that allow me to remove the conditional?

Answer 1

You can already get rid of the conditional. If the array is nil, then you'll be sending a message to nil, which is a no-op. The assignment to nil is pointless either way as well. And if you make it a retain property, explicitly releasing the old value is wrong.

However, there is one case where that code will not work correctly: When the argument is the current value. In that case, you'll release the current value and then try to use the released object (which may already have been dealloced) to create a new array.

Answer 2

You don't need the conditional at all; you can message nil (including a release), and nothing will happen. You also don't need to allocate a new array; you can retain the one passed to you instead. If you're worried about actually getting an NSMutableArray, you can make a copy. I'd do this:

- (void)replaceArray:(NSArray *)newArray
{
    [myArray autorelease];
    myArray = [newArray copy];
}

Or, if you don't want to use autorelease, you could do:

- (void)replaceArray:(NSArray *)newArray
{
    if (myArray != newArray) {
        [myArray release];
        myArray = [newArray copy];
    }
}

Answer 3

Imaging the following:

MyClass * myObj;
// init myObj
NSArray * array = [myObj myArray];
[myObj replaceArray:array];

In this case, myArray and newArray are the same, which means you're using it after it being released. To solve this problem, all you need to do is remove the replaceArray: method, and implement the property as @synthesize myArray. So the above code changes to

MyClass * myObj;
// init myObj
NSArray * array = [myObj myArray];
[myObj setMyArray:array];

and your problem is solved by the synthesized implementation.

Note that you are setting your value by creating a new array:

myArray = [[NSArray alloc] initWithArray: newArray];

if this is the behaviour you want, you should change your property definition to copy instead of retain:

@property (nonatomic, copy) NSArray myArray;

Answer 4

I've voted up mipadi because his answer is right in the context of the question you asked, but why not just use a property and do away with replaceArray: altogether:

@interface MyClass {
   NSArray   *myArray;
}

@property (copy) NSArray* myArray;

@end

@implementation MyClass

@synthesize myArray;

-(void) dealloc
{
    [myArray release];
    [super dealloc];
}

@end