---
title: 'iOS Code Review: Loose Guidelines'
teaser: |
  How to perform a code review of an iOS app using
  static analyzers and other tools.
tags: ios
author: Jack Nutting
published_on: 2014-02-19
---

From time to time I've been asked to do an independent code review, to determine
the overall health of a client's code base. Unlike a code walkthrough, where
someone familiar with the code shows the main components, this is a code review
where an outside expert examines the project, takes copious amounts of notes,
and reports back either in written form or in a meeting with the team, depending
on what the client wants.

This is separate from doing testing or any other sort of QA on the applications
themselves. The idea is that you might have an application that works great and
passes all kinds of acceptance tests, but is built in a such a way that future
maintenance and enhancements will be difficult and/or costly. If we can identify
problem areas in an application's source code, we can help set things on a
better course. The sooner we can discover potential problems, the better. The
experiences and guidelines described here are centered around iOS programming
practices, but many of them apply to other sorts of projects as well.

The last time I did this, the client's budget was limited, so there wasn't time
to do in-depth examination of every source code file in each of the clients's
application. I had a lot of territory to cover, and not a whole lot of time. So,
I decided to do it in two phases: First, I took an initial look at each project
to establish a quick (if superficial) opinion of each project's health. After
that, I dove deeper into each project, paying extra attention given to the
projects that set off the most warning flags during the initial phase. This
procedure worked pretty well, since I was able to report back with an
approximate health level for each application, plus a lot of specifics for those
that seemed to be in worse shape than the others.

The set of guidelines I'm outlining here are the kind of thing that anyone can
do on their own codebase as well. If you don't understand what some of these
points are all about, or why they're worth thinking about when it comes to your
own apps, this could be a good time to improve your own skills a bit by thinking
about some of these topics and looking for relevant discussion and debate (for
example, on the internet).

## Phase One: A Quick Look

To get a feel for the overall health of each app, do the following things:

- Make sure all external resources required by the project (3rd-party code, etc)
  are fully contained within the app, or referenced through submodules, or (best
  of all) included via [CocoaPods][cocoapods]. If they're not managed in some
  way (e.g. CocoaPods) see if there is any documentation describing how to
  obtain and update these resources.
- Open the project in Xcode and build it. Make sure the project builds cleanly
  (with no warnings, and hopefully errors).
- Perform a [static analysis][static_analysis] in Xcode to see if any other
  problems show up.
- Run [oclint][oclint] and see if this uncovers any other problems that Xcode's
  static analysis didn't reveal.
- Examine the project structure. Do the various source code files seem to be
  placed in a reasonable hierarchy? The larger the project is, the more
  important it becomes to impose some kind of structure, in order to help
  outsiders find their way around.
- Does the app have unit tests or integration tests? If so, run them and make
  sure they complete without any failures. Bonus points if tools are in use for
  measuring test coverage.

Doing all that should take several minutes for each app, regardless of the app's
size, unless you encounter major problems somewhere along the line. Finding
things that are not to your liking on one or two of those points doesn't
necessarily mean that you've got a huge problem on your hands, but by
considering your findings here you can start to get a sense of any project's
overall "smell".

## Phase Two: Diving Deep

After that, do a closer examination of each app, starting with the ones that set
off the most warning flags in your head during the initial examination. You
should look at every source code file (if you have the time to do so), reading
through the code with all of these things in mind. You'll probably want to take
notes as you go along, when you find things that need improving.

### Objective-C

- Are the latest Objective-C features from the past few years being used? This
  includes implicit accessor and instance variable synthesis, new syntactic
  shortcuts for creation of `NSNumber`/`NSArray`/`NSDictionary`, new syntax for
  array and dictionary indexing, etc.
- Are instance variables and properties used in a consistent way? Does the code
  use accessors where appropriate, keeping direct instance variable access
  limited to init and dealloc methods?
- Are properties declared with the correct storage semantics? (e.g. copy instead
  of strong for value types)
- Are good names used for classes, methods, and variables?
- Are there any classes that seem overly long? Maybe some functionality should
  be split into separate classes.
- Within each class, are there many methods that seem too long, or are things
  split up nicely? Objective-C code is, by necessity, longer than the
  corresponding code would be in a language like Ruby, but generally shorter is
  better. Anything longer than ten or fifteen lines might be worth refactoring,
  and anything longer than 30 or 40 lines is almost definitely in need of
  refactoring.
- Is the app compiled with [ARC, MRR][arc_mrr], or a mix? If not all ARC, why
  not?

### Cocoa

- Does the app make good use of common Cocoa patterns, such as [MVC][mvc],
  [notifications][notifications], [KVO][kvo], lazy-loading, etc? Are there any
  efforts underway to adopt patterns that aren't backed by Apple, but are
  gaining steam in the iOS world, such as [Reactive Cocoa][reactive_cocoa] and
  [MVVM][mvvm]?
- Are there view-controllers that are overloaded with too much responsibility?
- If discrete sections of the app need to communicate with each other, how do
  they do so? There are multiple ways of accomplishing this (KVO, notifications,
  a big pile of global variables, etc), each with their own pros and cons.

### Model Layer

- If the app is using Core Data, does the data model seem sufficiently
  normalized and sensible? Is the Core Data stack set up for the possibility of
  doing some work on a background thread? See [Theo][theo]'s [guide to core data
  concurrency][core_data_currency] for more on this.
- If not using Core Data, does the app store data using some other techniques,
  and if so, does it seem reasonable?
- At the other end of the spectrum, does the app skip model classes to a large
  extent, and just deal with things as dictionaries?

### GUI

- Is the GUI created primarily using xibs, storyboards, or code?
- Is GUI layout done with constraints, the old springs'n'struts, or hard-coded
  frame layout?
- Does the running app have a reasonable look and feel?

### Network Layer

- Is all networking done using asynchronous techniques, allowing the app to
  remain responsive?
- If a 3rd-party network framework is being used, is it a modern, supported
  framework, or something that's become a dead end?
- If no 3rd-party network framework is in use, are Apple's underlying classes
  being used in a good way? (There are plenty of ways to get this wrong).
- Does the app function in a fluid manner, without undesireable timeouts or
  obvious network lags affecting the user?

### Other

- Is the app localized for multiple languages? If so, is this done using
  standard iOS localisation techniques?
- If there are tricky/difficult things happening in the code, are these
  documented?

This is a pretty hefty set of things to consider. Depending on the code base,
you won't necessarily be able to find a clear yes/no answer for all of these
questions, and for certain types of apps, some of these points will be
meaningless. Note that I'm not saying exactly what I think are the "right"
answers for all of the questions listed here, although I certainly have my share
of strong opinions about most of these (but those are topics for other blog
posts). Even if you wouldn't agree with my answers, these are probably all
points that are worth thinking about and discussing with co-workers and other
collaborators to figure out just what seems right for *your* projects.

[cocoapods]: http://cocoapods.org/
[static_analysis]: https://developer.apple.com/library/ios/recipes/xcode_help-source_editor/chapters/Analyze.html
[arc_mrr]: https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/MemoryMgmt/Articles/MemoryMgmt.html
[mvc]: https://developer.apple.com/library/ios/documentation/General/Conceptual/DevPedia-CocoaCore/MVC.html
[notifications]: https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Classes/NSNotificationCenter_Class/Reference/Reference.html
[kvo]: https://developer.apple.com/library/mac/documentation/cocoa/conceptual/KeyValueObserving/KeyValueObserving.html
[reactive_cocoa]: https://github.com/ReactiveCocoa/ReactiveCocoa
[mvvm]: http://www.teehanlax.com/blog/model-view-viewmodel-for-ios/
[theo]: https://twitter.com/theocalmes
[core_data_currency]: https://thoughtbot.com/blog/core-data
[oclint]: http://oclint.org/