Proper way to copy a readonly NSMutableArray

Question

I have an object with a readonly property that I am trying to implement NSCopying for. It has a mutableArray called "subConditions" (which holds "SubCondition" objects). I have made it readonly because I want callers to be able to change the data in the array, but not the array itself. This worked really well until it was time to write the -copyWithZone: method.

After fumbling around a bit, I managed to get something that seems to work. I am not sure if it is the best practice though. Here is a simplified version of my -copyWithZone: method:

-(id)copyWithZone:(NSZone*)zone
{
    Condition *copy = [[[self class]allocWithZone:zone]init];


 NSArray *copiedArray = [[NSArray alloc]initWithArray:self.subConditions copyItems:YES];
 [copy.subConditions setArray:copiedArray];
 [copiedArray release];

    return copy;
}

Is this the correct/best way to copy a readonly mutableArray?

Answer 1

I have made [the mutable array property] readonly because I want callers to be able to change the data in the array, but not the array itself.

Mutating the value of an object's property without its knowledge is bad mojo. As an example, suppose that the object has an index set by which it knows which objects are selected; if another object then removes some sub-conditions from the array, some or all of the indexes in the set may refer to wrong objects or no objects at all, which means that accessing the selected sub-conditions would cause an NSOutOfRangeException.

This is a bad habit, and the solution is to switch to the inverse habit, which is to never do this. Always make changes to an owned array by telling its owner to make the changes—or, at least, make your own array with the changes applied and show that to the original array's owner. The owner (Condition) will, in that latter solution, probably wipe out its index set and forget the selection, but that's still better than causing exceptions.

Once you change the property's type from NSMutableArray to NSArray, the above code should give a warning, since only NSMutableArrays respond to setArray:. (The code will still work, presuming the subConditions accessor returns the mutable array and not an autoreleased copy, but the compiler will give a warning about it.) With mutating the array off the table, the problem becomes how to enable the Condition class to supply a copy of a Condition instance with a copied array of copied sub-Conditions while not allowing other classes to do it.

One solution is to simply store the new array directly into the copy's instance variable. Normally, this, too, would be bad mojo, but in this specific context (copyWithZone:, with the affected object being the copy), this is one of the very few cases where it is appropriate. Use the pointer-to-member operator to do that:

copy->subConditions = [[NSMutableArray alloc]initWithArray:self.subConditions copyItems:YES];

The other solution is to use a class extension to re-declare the property as readwrite within the class's implementation file. (Keep the readonly declaration in the class's header file.) Then you can use a property access message:

copy.subConditions = [[[NSArray alloc]initWithArray:self.subConditions copyItems:YES]autorelease];

There's not much to recommend one over the other, except that the direct ivar access is slightly faster: The property version creates a temporary array, and sends an accessor message to make the new Condition create its copy of that array. You may want to use the property access version first, then profile your application using Instruments to determine whether the overhead matters on hardware you care about.