Best Practices for Objective-C Coding
Preface
I don’t normally post highly technical stuff to my blog, but here’s an exception that I hope will benefit the Mac and iOS (iPhone & iPad) developer community. So if you’re not part of that, feel free to skip it.
Introduction
This article is a cumulative list of the most commonly-violated best practices for Objective-C coders that I’ve witnessed in my many years of experience with the language. I call them “commandments” because there are many good reasons to observe them and few, if any valid reasons not to. However, when I show these practices to other developers, they frequently raise one big objection…
The Big Objection: Won’t That Hurt Performance?
This is almost never a legitimate objection. If you can show me any case where code written in conformance to any of the following policies has become the performance bottleneck, you have my blessing to optimize that code with an exception to the policy, as long as how you’re breaking the policy and why are clearly documented in the code.
Why?
These policies target code safety, clean memory management, understandability, and maintainability. Most of them are not performance enhancers. Nonetheless, writing code with performance in mind is never the way to go first. Always write clean, correct code and then optimize only when and where profiling indicates you need it. In a typical UI-driven application, the single biggest bottleneck is almost always the user, followed by network access, and then disk access. Nonetheless, premature optimization is rampant among programmers who are concerned that if they don’t use every optimization trick they’ve been taught, their application will be dog slow. This simply isn’t true.
Important General Advice
Even if you understand how your code operates when you write it, that doesn’t mean others will understand it so completely before they attempt to modify it, and it doesn’t mean that you will understand it yourself when you revisit it months later. ALWAYS strive to make your code self-documenting by using verbose symbol names and add detailed comments in sections of code where meaning is not obvious despite clear and descriptive symbols. Any time you need to work around a bug or misfeature in an API you are using, isolate your workaround to a single method, explain your workaround and put a keyword like KLUDGE in your comments so you can easily find and fix these trouble spots later.
Thou Shalt…
ALWAYS use the @private directive before any instance variable declarations, and NEVER access data members directly from outside the class.
Why?
- Preserves information hiding (encapsulation)
- Limits to methods in the class implementation cases where data members are accessed directly.
- The default privacy for instance variables is @protected, which means subclasses can access these instance variables freely. But there has to be a very good reason to allow subclasses to do this— everything that a superclass exposes to the outside world becomes part of its contract, and the ability to change the internal representation of a class’s data without changing its API or contract is an important benefit of object-oriented encapsulation. By keeping your instance variables private, you make it clear that they are implementation details and not part of your class’s API.
Bad
Better
Thou Shalt…
ALWAYS create a @property for every data member and use “self.name” to access it throughout your class implementation. NEVER access your own data members directly.
Why?
- Properties enforce access restrictions (such as readonly)
- Properties enforce memory management policy (retain, assign)
- Properties are (rarely) used as part of a thread safety strategy (atomic)
- Properties provide the opportunity to transparently implement custom setters and getters.
- Having a single way to access instance variables increases code readability.
Bad
Better
Thou Shalt…
ALWAYS use the “nonatomic” attribute on your properties, unless you are writing a thread-safe class and actually need access to be atomic, and then put a comment in the code that this was your intent.
Why?
Classes with properties that aren’t declared “nonatomic” may give the impression that they were designed with thread-safety in mind, when in fact they weren’t. Only drop the “nonatomic” qualifier from properties when you have actually designed the class for thread-safety.
Bad
Better
Better
Thou Shalt…
NEVER allow your instance variable names to be confused with property names, or with data member names. ALWAYS end your instance variable names with an underscore. UNLESS you are subclassing a 3rd-party class which already has a data member of the same name, in which case pick another non-similar name or add another underscore and put a comment in as to why you did this.
Use “@synthesize name = name_;” in your implementation, instead of just “@synthesize name;”
Why?
- Even within the class implementation you have to have a very good reason to access a data member directly, instead preferring property accessors.
- Apple uses “_name” for their private data members, and by using “name_” for yours, you avoid naming conflicts.
- Apple has begun to use the “name_” convention in code examples and templates that demonstrate developer-level code.
Bad
Better
Thou Shalt…
NEVER redundantly add the data member to the class @interface yourself. Allow the @synthesize directive to implicitly add the data member.
Following this practice will yield many classes that explicitly declare no instance variables in the @interface section. When a class has no data members, omit the @private declaration, and even omit the opening and closing braces of the data member section.
Why?
- Reduces redundancy.
- Simplifies class headers.
- Avoids the need for forward class declarations in public headers merely so you can declare members of classes that are only actually used in the implementation.
Bad
Better
You may still need to declare the underlying name of the variable if you need to access it directly, as when writing custom getters and setters:
Won’t Work
Will Work
@property(nonatomic, retain) NSObject* myObj;
@end
// …
@implementation Foo
@synthesize myObj = myObj_;
- (NSObject*)myObj
{
return myObj_; // No problem.
}
- (void)setMyObj:(NSObject*)myObj
{
// no problem
[myObj retain]; // retain the argument
[myObj_ release]; // release the instance variable
myObj_ = myObj; // do the assignment
}
@end
Thou Shalt…
NEVER access a data member through an underscore-suffixed symbol UNLESS you are writing a setter or getter.
Bad
@property(nonatomic, retain) Bar* myObj;
@end
// …
@implementation Foo
@synthesize myObj = myObj_;
- (void)someMethod
{
myObj_ = [[Bar alloc] init];
}
@end
Better
@property(nonatomic, retain) Bar* myObj;
@end
// …
@implementation Foo
@synthesize myObj = myObj_;
- (void)someMethod
{
self.myObj = [[[Bar alloc] init] autorelease];
}
@end
Thou Shalt…
NEVER declare internal (private) methods or properties in the class header files. ALWAYS put all internal method and property declarations into a “class extension” in the implementation file.
Bad
// Foo.h
//
@interface Foo : NSObject
@property(nonatomic) int myPublicProperty;
@property(nonatomic, retain) Bar* myPrivateProperty; // This can be accessed by anyone who includes the header
- (int)myPublicMethod;
- (int)myPrivateMethod; // So can this.
@end
Better
// Foo.h
//
@interface Foo : NSObject
// Only the public API can be accessed by including the header
@property(nonatomic) int myPublicProperty;
- (int)myPublicMethod;
@end
// Foo.m
//
@interface Foo () // This is a "class extension" and everything declared in it is private, because it’s in the implementation file
@property(nonatomic, retain) Bar* myPrivateProperty;
- (int)myPrivateMethod;
@end
@implementation Foo
// …
@end
Thou Shalt…
ALWAYS call -autorelease on any instance handed back to you from an init-constructor.
Why?
Autorelease pools establish a deallocation policy for the instance at the point of allocation. Failing to do this is likely to result in memory leaks if you forget to manually call -release on the instance. As code gets modified and moved around by other programmers, the -release call is prone to get moved, deleted, or just farther away from the allocation. This can be a particular problem in methods that have more than one exit point (i.e., return-statements.)
Bad
- (void)someMethod
{
Bar* bar = [[Bar alloc] init];
// … do a lot of stuff with bar…
// …
// …
// …
// …
// …
// …
[bar release]; // easy to forget about!
}
@end
Better
- (void)someMethod
{
Bar* bar = [[[Bar alloc] init] autorelease];
// … do a lot of stuff with bar…
// …
// …
// …
// …
// …
// …
// nothing to forget about here!
}
@end
Thou Shalt…
NEVER use more than one return-statement in a method, but only as the last statement of a method that has a non-void value-type.
In methods that have a non-void value-type, declare a variable for the value as the very first statement, and give it a sensible default value. Assign to it in code paths as necessary. Return it in the last statement. NEVER return it prematurely using a return-statement.
Why?
Premature return-statements increase the possibility that some sort of necessary resource deallocation will fail to execute.
Bad
- (Bar*)barWithInt:(int)n
{
// Allocate some resource here…
if(n == 0) {
// …and deallocate the resource here…
return [[Bar alloc] init] autorelease];
} else if(n == 1) {
// …and here…
return self.myBar;
}
// …and here.
return nil;
}
@end
Better
- (Bar*)barWithInt:(int)n
{
Bar* result = nil;
// Allocate some resource here…
if(n == 0) {
result = [[Bar alloc] init] autorelease];
} else if(n == 1) {
result = self.myBar;
}
// …and deallocate the resource here, you’re done!
return result;
}
@end
Thou Shalt…
Understand what NSAutoreleasePools are for, when they are created and destroyed for you, and when to create and destroy your own.
- Automatically by NSRunLoop on each pass
- Automatically by NSOperation
- By you, at the start and end of threads
- By you, whenever you must create and release a large number of objects before you’ll cede control back to the run loop.
Thou Shalt…
ALWAYS prefer class-level convenience constructors over init-constructors. All of the foundation framework container classes provide these.
Bad
Better
Thou Shalt…
ALWAYS offer class-level convenience constructors in the API of the classes you write.
Why?
Clients of your class can reap the benefits of being able to keep the previous commandment.
Bad
Better
- (id)initWithBar:(int)bar baz:(int)baz;
+ (Foo*)fooWithBar:(int)bar baz:(int)baz;
@end
Thou Shalt…
NEVER release something you didn’t init!
Thou Shalt…
ALWAYS let your properties do retaining and releasing for you, if needed. NEVER call -retain or -release on objects yourself, UNLESS you are writing a setter for a property with the (retain) attribute, then be VERY CAREFUL.
Why?
-release may (or may not) cause an instance to become deallocated, but does not remove the variable that contains its reference from the current scope. This increases the possibility that someone will write code after the -release call that references the now-dangling pointer.
Thou Shalt…
ALWAYS use “self.propertyName = nil;” to release an object, whether or not that object’s property uses the “retain” strategy.
Why?
This idiom can be used not matter what your memory management strategy is (which can change over time), and even works when assigning nil to a custom setter that does special resource deallocation behind the scenes (like invalidating an NSTimer.)
Thou Shalt…
ALWAYS have a -dealloc method if your class keeps even one object pointer.
Thou Shalt…
ALWAYS make sure that your dealloc method assigns nil to every self object property, even if your current policy doesn’t retain the object.
Bad
{
[foo release];
[bar release];
[baz release];
[super dealloc];
}
Better
{
self.foo = nil;
self.bar = nil;
self.baz = nil;
[super dealloc];
}
Thou Shalt…
ALWAYS write a custom getter for a property where you have a custom setter, and vice versa.
Why?
Getters and setters need to have symmetrical behavior about memory management, thread safety, and any other side effects they create. You should not rely on a synthesized getter or setter having the proper symmetrical behavior to any custom getter or setter you write. Therefore, if you’re going to provide either the getter or setter yourself, you should provide both.
Thou Shalt…
ALWAYS write a custom setter for a property if there’s something you must ALWAYS do when you release an object (such as invalidating a timer).
Bad
@synthesize myTimer;
- (void)dealloc
{
self.myTimer = nil; // Timer not invalidated, we could get called back if the timer fires after we’re dealloced!
[super dealloc];
}
@end
Better
@synthesize myTimer = myTimer_;
- (NSTimer*)myTimer
{
return myTimer_;
}
- (void)setMyTimer:(NSTimer*)timer
{
[timer retain];
[myTimer_ invalidate];
[myTimer_ release];
myTimer = timer;
}
- (void)dealloc
{
self.myTimer = nil; // Timer guaranteed not to fire after we’re gone!
[super dealloc];
}
@end
Thou Shalt…
When writing constructors, ALWAYS minimize or eliminate the amount of code that executes before [super init] is called.
Why?
In general, the call to the superclass’ constructor may fail, causing it to return nil. If that happens, then any initialization you do before the call to the super constructor is worthless or worse, has to be undone.
Bad
- (id)initWithBar:(Bar*)bar
{
[bar someMethod];
// other pre-initialization here
if(self = [super init]) {
// other initialization here
} else {
// oops! failed to initialize super class
// undo anything we did above
}
return self;
}
@end
Better
- (id)init
{
if(self = [super init]) {
// minimal initialization here
}
return self;
}
// Other methods that put a Foo into a usable state
@end
Thou Shalt…
When writing a UIViewController and not using a Nib file, ALWAYS create and setup your view hierarchy in -loadView, and never in -init. In your implementation of -loadView, ALWAYS call [super loadView] first. Your implementation of -loadView is the ONLY place you should ever assign to the view attribute.
Thou Shalt…
NEVER call -loadView yourself! The view attribute of a UIViewController loads lazily when it’s accessed. It can also be automatically purged in a low-memory situation, so NEVER assume that a UIViewController’s view is going to live as long as the controller itself.
Thou Shalt…
ALWAYS set up the views you need once, then show, hide, or move them as necessary. NEVER repeatedly destroy and recreate your view hierarchy every time something changes.
Thou Shalt…
NEVER call -drawRect on a UIView yourself. Call -setNeedsDisplay.
Thou Shalt…
ALWAYS avoid long compound operations in your code. Local variables are your friend.
Bad
Better
NSDictionary* dataModelDict = [defaults objectForKey:@"foo"];
NSMutableDictionary* listDict = [NSMutableDictionary dictionaryWithDictionary:dataModelDict];
Why?
- Self-documenting
- Easier to understand
- Easier to step through in debugger
Thou Shalt…
NEVER use method names like -drawUI when you aren’t really drawing anything. ALWAYS prefer names like -setupUI or -resetUI, especially if there’s the possibility the method can be called more than once.
Thou Shalt…
NEVER repeat yourself. Have a single, authoritative location for each piece of information or functionality, and deploy it throughout the application, even when it means touching others’ code. NEVER program via copy-and-paste.
Updated October 15, 2010
Thou Shalt…
NEVER call -stringWithString UNLESS you:
- Are converting an NSString to an NSMutableString.
- Are converting an NSMutableString to an NSString.
- Need to guarantee that an NSString* you’ve been handed is really immutable
- Really need a copy of an NSMutableString because you plan to modify them separately.
Why?
NSStrings are immutable. You should never have to copy an NSString unless you want a mutable copy, or gurantee that an NSString pointer you want to save is not already an NSMutableString (and thus might have its value changed by some other code later.) In fact, attempts to copy NSStrings simply bump up the string’s retain count and return the original string:
NSLog(@"foo:0x%x retainCount:%d", foo, [foo retainCount]);
NSString* bar = [NSString stringWithString:foo]; // bumps retain count and adds to autorelease pool
NSLog(@"bar:0x%x retainCount:%d", bar, [bar retainCount]);
NSString* baz = [bar copy]; // just bumps retain count
NSLog(@"baz:0x%x retainCount:%d", baz, [baz retainCount]);
// Outputs:
// foo:0xcb18f30 retainCount:1
// bar:0xcb18f30 retainCount:2
// baz:0xcb18f30 retainCount:3
Thou Shalt…
When overriding a method in a superclass, ALWAYS call the superclass’ implementation, even if you know that super’s implementation does nothing, unless you have a very good reason to not call super’s implementation, in which case you must document your reason in the comments.
Why?
- Super’s implementation might not ALWAYS do nothing in the future.
- Other classes may eventually interpose themselves between your class and the superclass, with non-empty implementations.
- Makes your code more self-documenting in that it is not possible to call a super implementation unless the method is an override.
Bad
- (void)awakeFromNib
{
// do my setup
}
@end
Better
- (void)awakeFromNib
{
[super awakeFromNib];
// do my setup
}
@end
Added October 15, 2010
Thou Shalt…
In conditional statements, NEVER treat pointers or numerical values as booleans.
Why?
Booleans have two values: true and false (by convention in Objective-C we use YES and NO.) Pointers, on the other hand, have a value that is an object, or nil. Numerical values have values that are some number, or zero. While zero and nil evaluate to boolean false in a conditional context, this is a form of implicit type coercion that can make the meaning of your code more difficult to understand. So explicitly test for nil when dealing with pointers, 0 when dealing with integers, 0.0 when dealing with floating point values, etc.
Bad
{
if(bar && baz && quux) {
// do something interesting
}
}
Better
{
if(bar != nil && baz && quux != 0.0) {
// do something interesting
}
}
Added October 15, 2010
Thou Shalt…
NEVER use conditional statements to check for nil when you don’t have to. In particular, understand that in Objective-C, the act of calling any method on a nil object reference is a no-op that returns zero (or NO, or nil), and write your code accordingly.
Bad
{
if(obj != nil) {
[obj retain];
}
if(obj_ != nil) {
[obj_ release];
}
obj_ = obj;
}
Better
{
[obj retain]; // Does nothing if obj == nil
[obj_ release]; // Does nothing if obj_ == nil
obj_ = obj;
}
As an aside, note that the order of operations in the setter above ensures that if “self.obj = self.obj;” (or its equivalent) is executed, and this property holds the only remaining reference to the object, we won’t accidentally cause the object’s retain count to drop to zero, which would cause an unintended deallocation and a dangling pointer.
