(Almost) every time you use retain in Objective-C/Cocoa, you really should be using copy. Using retain can introduce some subtle bugs, and copy is faster then you think…
A Bug Waiting To Bite
The problem with using retain to “take ownership” of an object is that someone else has a pointer to the same object, and if they change it, you will be affected.
For example, let’s say you have a Person class with a straightforward setter method:
- (void) setThingsToCallTheBossToHisFace:(NSArray*)newNames {
[thingsToCallTheBossToHisFace autorelease];
thingsToCallTheBossToHisFace = [newNames retain];
}
And you use it to initialize a few Person objects:
NSMutableArray *appropriateNames = [NSMutableArray arrayWithObject:@"Mr. Smith"];
[anIntern setThingsToCallTheBossToHisFace:appropriateNames];
//Salaried Employees can also be a bit more informal
[appropriateNames addObject:@"Joe"];
[aSalariedEmployee setThingsToCallTheBossToHisFace:appropriateNames];
//the wife can also use terms of endearment
[appropriateNames addObject:@"Honey"];
[appropriateNames addObject:@"Darling"];
[theBossesWife setThingsToCallTheBossToHisFace:appropriateNames];
The code looks good, it compiles without error, and it has a bug in it. Because setThingsToCallTheBossToHisFace: uses retain, each Person object’s thingsToCallTheBossToHisFace field is actually pointing to the exact same NSMutableArray. So adding “darling” to the list of names the wife can use also adds it to the intern’s vocabulary.
If copy was used instead, then each Person would have their own separate list of names, insulated from changes to the temporary variable appropriateNames.
A Sneaky Bug Too
This is a particularly insidious problem in Foundation/Cocoa, because mutable objects are subclasses of immutable objects. This means every NSMutableThing is also a NSThing. So even if a method is declared to take an immutable object, if someone passes in a mutable object by accident, there will be no compile-time or run-time warnings.
Unfortunately, there isn’t a good way to enforce that a method takes an object, but not a subclass. Because Foundation makes heavy use of class clusters, it’s very difficult to figure out if you have an immutable class, or it’s mutable subclass. For example, with:
NSArray *immutableArray = [NSArray array];
NSMutableArray *mutableArray = [NSMutableArray array];
[immutableArray isKindOfClass:[NSArray class]] is YES
[immutableArray isKindOfClass:[NSMutableArray class]] is YES
[mutableArray isKindOfClass:[NSArray class]] is YES
[mutableArray isKindOfClass:[NSMutableArray class]] is YES
[mutableArray isKindOfClass:[immutableArray class]] is YES
[immutableArray isKindOfClass:[mutableArray class]] is YES
Sad, but true.
copy Is Fast!
With nearly every immutable Foundation object, copy and retain are the same thing — there is absolutely no penalty for using copy over retain! The only time you would take a performance hit using copy would be if the object actually was mutable. And then you really do want to copy it, to avoid bugs!
The only exceptions I know of are: NSDate, and NSAttributedString.
But don’t just take my word for it! Here’s the snippet of code I used to test all this:
NSMutableArray *objects = [NSMutableArray array];
//add anything that can be made with alloc/init
NSArray *classNames = [NSArray arrayWithObjects:@"NSArray", @"NSColor", @"NSData", @"NSDictionary", @"NSSet", @"NSString", nil];
for(NSString *className in classNames) {
id obj = [[NSClassFromString(className) alloc] init];
if(obj)
[objects addObject:obj];
else
NSLog(@"WARNING: Could not instatiate an object of class %@", className);
}
//manually add objects that must be created in a unique way
[objects addObject:[[NSAttributedString alloc] initWithString:@""]];
[objects addObject:[NSDate date]];
[objects addObject:[NSNumber numberWithInt:0]];
[objects addObject:[NSValue valueWithSize:NSZeroSize]];
//test if retain and copy do the same thing
for(id obj in objects)
if(obj != [obj copy])
NSLog(@"copy and retain are not equvalent for %@ objects", [obj className]);
Best Practices
Get in the habit of using copy, anytime you need to set or initWith something. In general, copy is safer then retain, so always prefer it.
I believe it is best to try copy first. If an object can not be copied, you will find out about it the first time your code is executed. It will be trivial to substitute retain for copy. But it is much harder, and takes much longer, to discover that you should have been using copy instead of retain.
A program must be correct before it can be made to run faster. And we have seen there is no performance penalty for copy on most common objects. So it makes sense to try copy first, and then replace it with retain if it proves to be necessary through measurement. You will be measuring before you start “optimizing”, right? (I also suspect that if taking ownership of an object is a bottle-neck, then the right optimization is not to switch to retain, but to find a way to use a mutable object, or an object pool, to avoid the “take ownership” step altogether.)
Choose copy, unless you have a measurable justification for using retain.
UPDATE 2009-11-10: Obj-C 2.0 blocks have some peculiarities,
For this reason, if you need to return a block from a function or method, you must[[block copy] autorelease]it, not simply[[block retain] autorelease]it.
Very well done written article.
I did not think of this solution.
Thanks for sharing!
Comment by funkyboy — November 17, 2008 @ 10:59 am
In your article from:
http://vgable.com/blog/2008/11/14/prefer-copy-over-retain/
I think you have a design flaw. I think it would help also if you showed the definition of Person and the init method for it. But having said that I’m hoping I might be able to help.
From what I can tell, you are suffering from one of these two possibilities and I’m pretty sure it’s #2:
1) Your having all the instances of the Person objects end up sharing the MutableArray for ‘thingsToCallTheBossToHisFace’ because you’ve got it declared to be a class variable
or
2) The array that you pass in to the ‘set’ method is what is truely shared outside and your passing that same array to all the person instances, so unless you make a copy of the array your instances all end up pointing to the external array. So you have Jill, Bob and Joe all having their ‘set…’ method called with the outside array passed in and you replace the ‘thingsToCallTheBossToHisFace’ internal array with a pointer to that external array. (Which is what you are doing from your description and code above)
My opinion on what you should be doing
Instead in the alloc/init method for the Person class you should be setting that ‘instance variable’ to be a new instance of NSMutableArray and along with that your set method should not be replacing the internal implementation of your names array with some array passed in from outside, but should instead add the names from the external array to yours by iterating over that external array and adding the objects it points to to your array one by one and using retain on those objects.
If you iterate over the passed in array and just add the objects to your instances internal array, then each Person will end up having their own array and you will be able to see that they do.
Doing it that way will give each person will have their own array like you intend, and will have those arrays pointing to the string objects that they are supposed too.
The strings themselves may be pointed to by multiple arrays, so Jill may point to the Immutable strings @’Honey’ and @’Dear’ and Joe may also point to those also but may also point to @’Lover’. (The strings pointed too would depend on the strings passed in in the array to the ‘set…’ call.)
So based on my example right above, Jill’s array will have two objects being pointed too and Joe’s will have three objects being pointed too. Joe and Jill will each have their own instances of MutableArray. The thing they will share then is that their respective mutable arrays point to the same immutable strings in some cases.
In your example pointing to the same strings is not a concern though because the strings themselves are not being modified by any Person instance, just referenced.
If the string objects themselves were to be modified, then you’d still iterate over the passed in array on the ‘set…’ method, but instead of just adding the objects as you iterated, you would create a new object based on that object and then add that new object. In the case of needing to modified the string itself, you’d create a new mutable string (NSMutableString) via [[NSMutableString alloc] initWithString: …]. Having done that you will have still kept your instances array, and populated it with new instances based on the strings in the array that was passed in. You can’t use copy in that case b/c copy of a Immutable string instance gives a immutable string instance and you need to be able to modify the strings.
That said, unless you need a copy you shouldn’t call copy. The naming implies it’s logical use. If you are truly giving the responsibility for an object to another object then you use retain. You use copy to get a new object based on another and give it out to someone so that you can continue to use the old one as is without them depending on yours.
Your use of copy copied the wrong object and if you examine the situation further, you were wasting lots of time and space by not using the original array you would have created for the Person instances in their init’s, you ended up doing lots of copying you didn’t need too, etc.
Summary
Your example of setting the things to call the boss is flawed in that you allow the wholesale replacement of what should be an internal implementation detail. Your class api should have methods for adding or removing names to call the boss that add or remove from the internal implementation, not whole replace the object instance of the implementation detail. So an api like:
addNameToCallBoss: (NSString*) aName
— Adds the string to your internal array and retains it.
removeNameToCallBoss: (NSString*) aName
— Removes the string from your internal array that matches the passed in string and releases it
addAllTheseNamesToCallBoss: (NSArray*) names
— Iterates across the objects in the ‘names’ array that was passed in and then adds those objects to your intenal array and retains them.
etc.
The implementation of addAllTheseNamesToCallBoss: interates over the supplied names array and gets each object out and adds it to the internal array instance you have. You may retain those objects or copy them, but don’t have the array itself replaced. This provides true encapsulation. It also allows you to keep the api the same regardless of if you change to using a dictionary, set or whatever internally. It also allows you to use retain and copy with the correct semantic intent.
Doing what I suggest above allows you to actually get the benefits of retain, use copy in the correct situation where you actually do want copies and copy the correct objects. It also follows the Rule of Demeter.
If you have any questions or want me to clarify further feel free to email me and I’ll answer. Ideally I would have liked to put in a diagram of what I am describing in text to show Jill, Joe and the strings they point too and how if you replace the array you have with the one passed in, then each Person you pass that array too ends up pointing to the same array. That is the true failure and the correct way to deal with it is what I described above, not doing a copy. The copy is wasteful in time and space for the reasons I said above. A diagram would show all that so well.
Comment by Sam Griffith — November 21, 2008 @ 10:48 am
I cannot thank you enough for this post. You just saved me hours of pain trying to figure out what was wrong with my program. Thank you!
Comment by J. D. Harper — March 13, 2009 @ 4:08 pm
I’ve pretty much stopped writing setter/getter code entirely and have moved entirely to @property and @synthesize for what used to be setter/getter pairs.
Combined with class extensions, it also makes it terribly easy to declare externally readonly / internally readwrite properties.
Thus, I declare almost all primitive object typed — NSString, NSArray, etc… — properties as (copy).
Comment by bbum — May 18, 2009 @ 1:09 am
About the “A Sneaky Bug Too”, I tried this:
[immutableString isMemberOfClass:[NSMutableString class]]
and it is NO.
So I guess we can use “isMemberOfClass” to make sure that the Object is not a subclass, in case we know which subclass we will be worry about.
Comment by Bo — February 12, 2010 @ 10:27 pm
Congratulations! You have written the concept in a very intuitive way.
Thanks and Regards,
Marcos Vilela.
Comment by Marcos Vilela — March 4, 2011 @ 8:22 pm
Goog article.. thanks..
Comment by yuri — July 31, 2011 @ 12:47 pm
I blog often and I seriously thank you for your content.
This article has really peaked my interest. I am going to book mark your website and keep checking for new information about once per
week. I subscribed to your RSS feed as well.
Comment by tuxedo button adjustable — March 22, 2015 @ 4:14 pm