This document contains a set of recommended guidelines, conventions, and best practices for writing code in our iOS code base. The main motivation for this guide is to improve the readability of the code base. Remember that code is read much more often than it is written, and that 80% of the lifetime cost of software goes to maintainence.
Note that the purpose of this guide is not to convince any one that whatever listed below is the best way of doing something. The idea is to keep a set of recommended guidelines for everyone to follow in order to make the code more consistent, understandable, and less prone to unobvious pitfalls.
Note: Suggestions for improvements are highly encouraged.
Note: Please take a look and familiarize yourself with Google’s Objective-C Style Guide. For things not specified below, we will try to follow their conventions.
Space only. 4-space indents.
Make sure you have Automatically trim trailing whitespace and Including whitespace-only lines enabled in Xcode.
We will go with the default template Xcode uses: no spacing except between parameters and the one space after the -.
- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath
{
...
}
Brackets go on the same line as the conditions, e.g.
if (foobar) {
...
} else {
...
}
for (foo; foo; foo) {
...
}
Should still have braces
if (foobar) {
...
} else {
...
}
Please take a look at Apple's Coding Guidelines espcially on how methods should be named. e.g. use - (NSSize)cellSize instead of - (NSSize)getCellSize.
For pointers, place the star in front of the variable name. e.g. NSString *foo
Use Obj-C literals for dictionaries, arrays and numbers whenever possible: e.g. @42 instead of [NSNumber numberWithInt:42], and
NSDictionary *dictionary = @{
@”k1” : @”v1”,
@”k2” : @”v2”,
};
instead of [NSDictionary dictionaryWithObjectsAndKeys:@”v1”, @”k1”, @”v2”, @”k2”, nil] BAD
Use dot notations only for declared properties and structs.
Do NOT do stuff like foobar.release or foobar.count. BAD
Reasoning: while the dot syntax is more or less just syntactic sugar around getter/setter calls, it is idiomatic to use them only for properties.
ivars should always be prefixed by an underscore (e.g. NSString *_foobar)
Reasoning: default backing ivars for properties are also prefixed by underscore. We should be consistent.
Mark them as nonatomic/atomic, retain/assign explicitly.
Reasoning: makes life easier when migrating to/from ARC (where default is strong instead of assign)
If you are not using ARC, you should still annotate ivars that are weak. (in ARC, obviously you would need the __weak qualifier.) By default, we will assume ivars that are not marked as weak to be retained.
e.g. id<SomeDelegate> _delegate; // weak ref
External callers should never mutate ivars directly. Provide getters/setters for external callers.
Private ivars should go under the @implementation block. e.g.
@implementation
{
NSString *_foobar;
}
Keep #imports sorted alphabetically. Start with local imports, (for .m files, import its own headers first). followed by global imports. e.g. for DBFoo.m:
#import "DBFoo.h"
#import "DBFoo+Protected.h"
#import "DBAnotherFile.h"
#import "DBSomeOtherFile.h"
#import <CoolFramework/Header.h>
#import <FoobarFramework/Foo.h>
Use #pragma mark extensively to designate logical clusters of methods, such as those methods that implement a particular protocol, or the getter/setters of a particular object. For sections vs. subsections, you can use #pragma mark - to create dividers in Xcode’s symbol navigator. For example, in a table view controller you may find pragma marks like the following:
#pragma mark - Lifecycle // for init and dealloc methods
#pragma mark - Appearance
#pragma mark view initializations
#pragma mark view utilities
#pragma mark - UIViewController overrides
#pragma mark - UITableViewDataSource
#pragma mark - UITableViewDelegate
To keep things easy to find, methods should be grouped by functionality rather than scope. For example, a private class method can be in between two public instance methods.
Always place them first - dealloc should go first (and ivars should be released in the same order as they are declared).
For larger switch-case blocks, make each case block into a scoped block, e.g.
switch (foo) {
case 0: {
...
}
break;
...
}
Avoid using default as catch-alls if possible (e.g. don’t use default just to omit the last case). When possible, default should be an error case that contains an NSCAssert or DBLogError.
Prefix them with db_. e.g. - (void)db_flushQueue
Reasoning: makes it easy to tell if a method is private or not (e.g. when reading a diff that modifies a method, it is nice to know right away if the method is public/private). It also makes it less likely for subclasses to accidentally override a private method - because Apple uses
_to prefix their private methods.
If a private method doesn’t use any ivar, just make it a class method.
Reasoning: it is much easier to reason through a method if I don’t have to think about the states it could be in.
Label your TODOs and FIXMEs with names and dates:
TODO:(rich) 2013-09-13 finish the style guide!
Please follow the prefix style TODO:(<name>) <date> consistently to make things easier to grep.
Use FIXMEs for urgent things that need to be fixed before pushing.
Use static consts instead of #defines for constants.
static const CGFloat kFooBar = 123.0;
Reasoning: it respects scope, is type-safe, and is loaded as a symbol and therefore easier to access through debugger.
Unless certain API libraries specify exact types you should use.
Reasoning: Most of the Cocoa libraries prefer NSIntegers, so it is best to pass in what they expect when crossing API boundaries. Since
intvaries in size between architectures, this is more reliable for developing against the iOS libraries.
As introduced in 2012’s WWDC, you should use NS_ENUM to declare your enums.
typedef NS_ENUM(NSInteger, UITableViewCellStyle) {
UITableViewCellStyleDefault,
UITableViewCellStyleValue1,
UITableViewCellStyleValue2,
UITableViewCellStyleSubtitle
};
To make autocomplete easier, you should also name your constants <enum name><variant>.
Reasoning:
NS_ENUMgives stronger type checking, code completion.
Similar to NS_ENUM, use the new NS_OPTIONS construct in iOS 6, e.g.:
typedef NS_OPTIONS(NSUInteger, UIViewAutoresizing) {
UIViewAutoresizingNone = 0,
UIViewAutoresizingFlexibleLeftMargin = 1 << 0,
UIViewAutoresizingFlexibleWidth = 1 << 1,
UIViewAutoresizingFlexibleRightMargin = 1 << 2,
UIViewAutoresizingFlexibleTopMargin = 1 << 3,
UIViewAutoresizingFlexibleHeight = 1 << 4,
UIViewAutoresizingFlexibleBottomMargin = 1 << 5
};
Reasoning: Because ObjC doesn't support namespaces..
Use block-based methods like enumerateObjectsUsingBlock, or fast enumerations, e.g.
for (id setObject in collection) {
}
Reasoning: reduces the amount of context a reader needs to keep in his/her head. The reader doesn't have to worry about the correctness of index variables. Enumerators are also more performant in general.
We will use NSCAsserts instead of asserts or NSAsserts for doing assertions.
Reasoning:
NSAssertis a macro that actually internally referencesself. In other words, when used in a block, you may unintentionally cause the block to retainself. We could alternatively use NSAsserts in cases where we are actually okay with usingself, but sinceNSAssertandNSCAssertdo roughly equivalent things (they both include file name, method name + line number, thoughNSCAssertdoes not print the actually type ofself), we will keep things simple and always useNSCAssert.
We will conform with Cocoa’s policy where "Cocoa’s ownership policy specifies that received objects should typically remain valid throughout the scope of the calling method.” In other words, we should make sure that objects returned by a public method will not just disappear due to the caller then calling something else in the object.
For example, when a caller does something like this:
NSString *foo = a.foo;
a.foo = @”blah”;
NSLog(@”foo = %@”, foo);
We should have the implementation of setFoo: and foo ensure that the local variable foo remains valid till the end of the caller's scope and the NSLog doesn't crash.
To conform with the memory management policy mentioned above, we will always autorelease instead of release ivars when re-assigning. e.g.
- (void)setFoo:(NSString *)foo
{
[_foo autorelease];
_foo = [foo retain];
}
Reasoning: this is to avoid the case mentioned in the example above, where the caller assigns something to a local variable and then the object gets deallocated.
There are three workarounds for these issues: 1. we can make the local variable retain-autorelease the result of the getter; 2. we can make getters return something that’s retain-autoreleased; 3. or we can do what the above suggested.
The later two conform to Cocoa’s policy “Cocoa’s ownership policy specifies that received objects should typically remain valid throughout the scope of the calling method.” and the last one suggested is somewhat more robust.
The setter should follow the same semantics as the autorelease-then-retain rule above, e.g.
@property (nonatomic, retain, setter=setFoo:) NSNumber *foo;
Reasoning: this is similar to the rule above. The issue is that the default synthesized setter does not conform to the autorelease-then-retain pattern, so the example in the previous rule would break.
When returning an object from a Cocoa collection (e.g. NSArray), you should do a retain-autorelease. e.g.
- (id)foo
{
return [[_someArray[0] retain] autorelease];
}
Similarly, if you are using an object from something that is later released, the object may be released as well, so you need to protect it from being released.
Reasoning: when removing objects from collections like
NSArrays, those objects get released right away. So in cases where a caller is holding onto an object returned from these methods, if we then remove that object from the array, that reference will no longer be valid.
When you wish to reference a block literal after the scope of a method (e.g. by storing it as an ivar), you should explicitly copy the block by calling, e.g. [theBlock copy].
Reasoning: As an optimization, blocks and their storage starts out on the stack. Therefore, if you wish to reference a block after the current scope returns, you should copy it to the heap by calling
[theBlock copy]explicitly.
Reasoning: Unless you foresee a performance hit, you should autorelease when creating a local variable. Otherwise later changes may do a return prior to the release or add a branch that omit the release, causing a leak. For memory-aggresive code paths, wrap them with a tighter
@autoreleasepoolblock to drain the autoreleased objects.
Reasoning: To avoid triggering side effects of setters and getters.
Reasoning: Use copy instead of retain for NSStrings passed in from external callers in case they are passing in NSMutableStrings.
Common mistake: casting the length of an array into a BOOL
Reasoning: even if the number is > 0, it may still cast to NO if the low bits are 0.
Instead of declaring @propertys in your .m file, just use ivars.
Reasoning: properties don’t really give you much other than the setter/getters. and, as the rule above mentioned, you should use custom setters for retained properties anyway. The only thing you get may be KVO, which you shouldn’t need for private states.
One exception being the use of atomic properties, which you should use sparingly.
If an object has been set as the delegate/observer of other objects (e.g. notification center), you should nil them out in dealloc.
NOTE: Even in ARC code, where delegate properties should be weak, it is still important to nil out a delegate pointer pointing to self from a system class. That is because most of these are still compiled as assign.
Reasoning: otherwise, if the object that has you as the delegate later tries to talk to you, it would crash.
ivars are automatically initialized to nil or 0, so you do not need to explicitly nil them out, but not local variables. So you should always initialize them to something manually.
For instance, for protected methods in DBFooViewController, we should place the signatures in a separate file DBFooViewController+Protected.h. The .m file should import both the DBFooViewController.h and DBFooViewController+Protected.h files.
The methods should be declared in a class extension and not a category, and the implementation should still belong to the primary implementation for the class.
Reasoning: this way when callers import your header, they won’t get the protected methods and won’t accidentally call protected methods. Only subclasses should have to import the protected header.
Calling self.view will implicitly load the view, which is usually not necessary.
For simple callback patterns, we prefer blocks over the target-selector delegate pattern.
e.g. instead of using an UIActionSheet, setting the delegate and then checking in the delegate which action sheet and which button is clicked, you should just use the block-based alternative that assigns a callback block to each button.
Reasoning: helps reduce code separation, e.g. imagine some async op with a completion handler. Instead of having a separate delegate method that implements the handler, it is much clearer what happens if you do something like:
[op startWIthCompletionHandler:^() {
// ... stuff to be done after the op finishes ...
}];
By default, we will assume all methods are only going to be called from the main thread. If a method is expected to be called from a background thread (e.g. certain Apple notifications like ALAssetsLibraryChangedNotification), make sure to add comments accordingly and add assertions if possible (e.g. DBAssertMainThread).
Similarly, we will assume all instance variables to be accessed only via the main thread. If certain ivars do need to be shared across threads (or are accessed exclusively from a specific dispatch queue), make sure you add comments accordingly.
For classes that are designed to have a thread-safe interface, make sure you add documentations around how things work.
By default, all properties are atomic, so you should mark them explicitly as nonatomic.
Reasoning: making a property atomic means all setters/getters are synchronized and has the perf hit of being so. Also, it is rare for atomic to actually be useful - most of the time if your variables need to be shared across thread (which is hopefully very rare), you probably don't just want the setting and accessing of the individual variables to be protected.
And you should almost never use performSelectorInBackground.
Reasoning: the reasoning is pretty similar to why we prefer blocks over target-selectors. This also makes the code easier to reason about as we won’t be forced to have separate methods that are called on a background thread.
Use forward declaring instead if possible e.g. @class DBFoo.
Reasoning: reduce compile time and unnecessary dependencies
Identify clearly your class's designated initializers that other initializers call through.
Reasoning: so subclasses can just override those to guarantee that the subclass' initializer is called.
If callers shouldn’t call them any more, make them throw an assert.
[[NSNotificationCenter defaultCenter] addObserver:self
selector:@selector(db_updateStuffWithNotification:)
name:@"SomeNotificationName"
object:theSender];
Reasoning: while it is not required (e.g. supplying a selector that doesn't take in any argument will still work), doing it this way makes it obvious to the reader that the method is called as a result of a notification.
Blocks implicitly creates strong references to self if they access self or an ivar by reference.
In other words, it is possible to accidentially introduce permanent retain cycles when using blocks. A common scenario is, for instance, when you use a BlockAlertView and give it an observer block that retains self. Assuming that self retains the alert view, this may cause a retain cycle where self -> alert view -> block -> self.
(Quick reference from Apple regarding when things are retained by blocks: Blocks and Variables.)
To avoid these cycles, we can do what is typically known as the "strong-weak dance":
__weak DBFoo *weakSelf = self;
[_someRandomViewIOwn setObserverBlock:^{
DBFoo *strongSelf = weakSelf;
// NOTE: strongSelf may point to nil if weakSelf is already nil'ed out prior to the block being called!
[strongSelf doSomeStuff];
}];
Reasoning: The idea is that, by using
weakSelf, the block no longer retainsself. However, it is still possible forweakSelfto get nil'ed out before the block is invoked. To make things even worse, if the object may be touched by multiple threads, it is possible forweakSelfto nil out in the middle of the block!
To preventselffrom disappearing in the middle of the block, we addedstrongSelfto artificially create a strong reference toweakSelf. WhilestrongSelfmay still point to nil, it will be consistent throughout the scope of the block, so you don't have to worry about it disappearing arbitrarily.
You also should not, however, abuse this pattern. A lot of times when you just want to, say, run a block later (e.g. animations, dispatch_after), you do NOT need to do this - it is fine so long if those retain cycles are temporary.
In non-ARC land, we do not have weak references that nil themselves out, so the alternative will be doing something like __block to mark variables are block variables. e.g.
__block DBFoo *weakSelf = self;
[_someRandomViewIOwn setObserverBlock:^{
// NOTE: if self is deallocated before this is invoked, deref'ing weakSelf will crash!
[weakSelf doSomeStuff];
}];
Since weakSelf will not nil itself out here, for objects that do this, if you think the object holding onto the block may have a longer life time than self, you should make sure you nil out the observer block in dealloc (similar to what you should do in a delegate pattern).
Another scenario where self may get nil'ed out
TODO:(rich) 2013-09-20 fill in the explanations for the rest of these bugs..
e.g. if you are creating a thread that just continuously iterate on a while loop, at the minimum, the pool should drain after each iteration.
For example:
+ (void)initialize
{
if (self != [TheClass class]) return;
…
}
Reasoning: If subclasses didn’t override the method, the superclass’s implementation may fire unnecessarily.
Unless you really want to override the existing behaviors, don’t forget to call the original implementation. Especially for UIKit classes
Accessing the backing ivar will bypass the lock that synchronizes its getter/setters.
Make sure people understand why your hacks are needed! If people are later modifying your code or trying to reuse your code for other platforms, they will need to understand your reasoning so they can determine whether the hacks are still relevant. When developing against frameworks we do not control, it is not unlikely that hacks needed for one version will break or become irrelevant in the next.
A good rule of thumb is to consider the amount of time it takes for a person with zero context to understand/modify your code with vs. without comments.
Let's call the amount of time needed to write your comments A, and the amount of time need for a person to understand/modify your commented code B vs. uncommented code C. Please take the time to add comments if A + N*B < N*C - where N is the number of people who will read your code. =)
Also remember that your code is a type of documentation and is meant to be read by fellow human beings! When you find yourself writing a lot of comments about a chunk of code, you should also consider rearchitecting or breaking down the code.
Try to keep your methods short and easy to reason about. Break methods up by functionalities!
Method names should reflect what they do instead of what is calling them.
Consumers of a class should only be concerned with what the class promises to do. The more internal details you expose, the more things are likely to break when you modify the implementation of your code.
When possible, properties should be readonly. Or better yet, don’t expose them at all!
When creating new methods, try to make it so readers do not have to dig into the documentation or implementation to figure out what something means.
For instance, do NOT do stuff like: - (void)refresh:(BOOL)animated; BAD. Although it is clear in the implementation that the flag is referring to whether the refresh must happen with animation, in the caller side, you will see code that looks like this: [foobar refresh:YES] BAD, where it is impossible to figure out by reading what the parameter refer to.
To mitigate this, you should name your methods so the word prior to the param indicates what it is expecting. To be even clearer, you may also use an enum for the parameter. e.g. - (void)refreshWithTransitionType:(DBFoobarTransitionType)transitionType;.
A pretty good read on this subject: hall of api shame: boolean trap.
It is impossible to design things to support every foreseeable use case, but you should try to document any known issues. For instance, if a view controller does not lay out its subviews properly in landscape mode, leave a comment so people wishing to reuse it later will know.
If a property declared as a more generic type is known to be of a specific subclass, redeclare the property as the specific type and implement the getter to consolidate the cast in one location, rather than litering casts throughout the code.
When reading code, it is usually best if variables are declared and used in the smallest scopes possible, so it is easier to read and reason through the effects of the variables (where they are used, modified, etc.).