There are several ways to write setters for Objective-C/Cocoa objects that work. But here are the practices I follow; to the best of my knowledge they produce the safest code.
Principle 0: Don’t Write a Setter
When possible, it’s best to write immutable objects. Generally they are safer, and easier to optimize, especially when it comes to concurrency.
By definition immutable objects have no setters, so always ask yourself if you really need a setter, before you write one, and whenever revisiting code.
I’ve removed many of my setters by making the thing they set an argument to the class’s -initWith: constructor. For example,
CustomWidget *widget = [[CustomWidget alloc] init]; [widget setController:self];
becomes,
CustomWidget *widget = [[CustomWidget alloc] initWithController:self];
This is less code, and now, widget is never in a partially-ready state with no controller.
It’s not always practical to do without setters. If an object looks like it needs a settable property, it probably does. But in my experience, questioning the assumption that a property needs to be changeable pays off consistently, if infrequently.
Principle 1: Use @synthesize
This should go without saying, but as long as I’m enumerating best-practices: if you are using Objective-C 2.0 (iPhone or Mac OS X 10.5 & up) you should use @synthesize-ed properties to implement your setters.
The obvious benefits are less code, and setters that are guaranteed to work by the compiler. A less obvious benefit is a clean, abstracted way to expose the state values of an object. Also, using properties can simplify you dealloc method.
But watch out for the a gotcha if you are using copy-assignment for an NSMutable object!
(Note: Some Cocoa programmers strongly dislike the dot-syntax that was introduced with properties and lets you say x.foo = 3; instead of [x setFoo:3];. But, you can use properties without using the dot-syntax. For the record, I think the dot syntax is an improvement. But don’t let a hatred of it it keep you from using properties.)
Principle 2: Prefer copy over retain
I covered this in detail here. In summary, use copy over retain whenever possible: copy is safer, and with most basic Foundation objects, copy is just as fast and efficient as retain.
The Preferred Pattern
When properties are unavailable, this is my “go-to” pattern:
- (void) setX:(TypeOfX*)newX;
{
[memberVariableThatHoldsX autorelease];
memberVariableThatHoldsX = [newX copy];
}
Sometimes I use use retain, or very rarely mutableCopy, instead of copy. But if autorelease won’t work, then I use a different pattern. I have a few reasons for writing setters this way.
Reason: Less Code
This pattern is only two lines of code, and has no conditionals. There is very little that can I can screw up when writing it. It always does the same thing, which simplifies debugging.
Reason: autorelease Defers Destruction
Using autorelease instead of release is just a little bit safer, because it does not immediately destroy the old value.
If the old value is immediately released in the setter then this code will sometimes crash,
NSString* oldName = [x name]; [x setName:@"Alice"]; NSLog(@"%@ has changed their name to Alice", oldName);
If -setName: immediately releasees the value that -name returned, oldName will be invalid when it’s used in NSLog.
But if If -setName: autorelease-ed the old value instead, this wouldn’t be a problem; oldName would still be valid until the current autorelease pool was drained.
Reason: Precedent
This is the pattern that google recommends.
When assigning a new object to a variable, one must first release the old object to avoid a memory leak. There are several “correct” ways to handle this. We’ve chosen the “autorelease then retain” approach because it’s less prone to error. Be aware in tight loops it can fill up the autorelease pool, and may be slightly less efficient, but we feel the tradeoffs are acceptable.
- (void)setFoo:(GMFoo *)aFoo { [foo_ autorelease]; // Won't dealloc if |foo_| == |aFoo| foo_ = [aFoo retain]; }
Backup Pattern (No autorelease)
When autorelease won’t work, my Plan-B is:
- (void) setX:(TypeOfX*)newX;
{
id old = memberVariableThatHoldsX;
memberVariableThatHoldsX = [newX copy];
[old release];
}
Reason: Simple
Again, there are no conditionals in this pattern. There’s no if(oldX != newX) test for me to screw up. (Yes, I have done this. It wasn’t a hard bug to discover and fix, but it was a bug nonetheless.) When I’m debugging a problem, I know exactly what setX: did to it’s inputs, without having to know what they are.
On id old
I like naming my temporary old-value id old, because that name & type always works, and it’s short. It’s less to type, and less to think about than TypeOfX* oldX.
But I don’t think it’s necessarily the best choice for doing more to old than sending it release.
To be honest I’m still evaluating that naming practice. But so far I’ve been happy with it.
Principle 3: Only Optimize After You Measure
This is an old maxim of Computer Science, but it bears repeating.
The most common pattern for a setter feels like premature optimization:
- (void) setX:(TypeOfX*)newX;
{
if(newX != memberVariableThatHoldsX){
[memberVariableThatHoldsX release];
memberVariableThatHoldsX = [newX copy];
}
}
Testing if(newX != memberVariableThatHoldsX) can avoid an expensive copy.
But it also slows every call to setX:. if statements are more code, that takes time to execute. When the processor guesses wrong while loading instructions after the branch, if‘s become quite expensive.
To know what way is faster, you have to measure real-world conditions. Even if a copy is very slow, the conditional approach isn’t necessarily faster, unless there is code that sets a property to it’s current value. Which is kind of silly really. How often do you write code like,
[a setX:x1]; [a setX:x1]; //just to be sure!
or
[a setX:[a x]];
Does that look like code you want to optimize? (Trick question! You don’t know until you test.)
Hypocrisy!
I constantly break Principle 3 by declaring properties in iPhone code as nonatomic, since it’s the pattern Apple uses in their libraries. I assume Apple has good reason for it; and since I will need to write synchronization-code to safely use their libraries, I figure it’s not much more work to reuse the same code to protect access to my objects.
I can’t shake the feeling I’m wrong to do this. But it seems more wrong to not follow Apple’s example; they wrote the iPhone OS in the first place.
If you know a better best practice, say so!
There isn’t a way to write a setter that works optimally all the time, but there is a setter-pattern that works optimally more often then other patterns. With your help I can find it.
UPDATE 03-30-2009:
Wil Shiply disagrees. Essentially his argument is that setters are called a lot, so if they aren’t aggressive about freeing memory, you can have thousands of dead objects rotting in an autorelease pool. Plus, setters often do things like registering with the undo manager, and that’s expensive, so it’s a good idea to have conditional code that only does that when necessary.
My rebuttal is that you should optimize big programs by draining autorelease pools early anyway; and that mitigates the dead-object problem.
With complex setters I can see why it makes sense to check if you need to do something before doing it. I still prefer safer, unconditional, code as a simple first implementation. That’s why it’s my go-to pattern. But if most setters you write end up being more complex, it might be the wrong pattern.
Really you should read what Wil says, and decide for yourself. He’s got much more experience with Objective-C development then I do.
