Storyboard Code Review Tutorial

It’s extremely important to keep your storyboards and xibs in tip top shape. Just like code, one of the easiest ways to do this is through storyboard code review. I heard the question posed on the iOhYes podcast, episode #108, “What’s involved with storyboard code review?” I’m proud of the team that I work on, and I think we’ve come up with a fairly mature process of storyboard code review. Here’s what we look for:

Most Important Thing – Small Commits

The first thing I look for, is the commit small enough such that it adds one concise piece of functionality to the app in an end to end fashion? Or does the commit add multiple things, or even partial implementations? This goes beyond just storyboards, so you can apply this to all aspects of code reviewing a commit. This fundamental building block for structuring your commits to be atomic, small, and concise will facilitate an easier code review. We strive for less than 500 changed lines of code per commit not including storyboard, project files, or added dependencies (yes, we check in our CocoaPods and you should too).

By keeping your commit small, reviewers of the code will not be overwhelmed with what they are looking at. Things will not slip through the cracks. And it will be all the more obvious as to “what’s changed in this commit?”

Happy Auto Layout

Assuming you are using Auto Layout (and if you’re not, you should be), another important thing to look for during storyboard code review is whether Auto Layout is correctly defined in the storyboard. Specifically, Auto Layout should know how to position every item in the storyboard. This is not pedantic, it’s correct. Remember, bugs come from incorrect Auto Layout. You don’t want bugs do you?

Auto Layout constraints should be:

1. Comprehensive for each UIView – nothing ambiguous
2. Correct relative to how the UIView is placed – nothing misplaced

There’s two ways to this. First, my preferred way:

Look for either misplaced="YES" or ambiguous="YES" in the storyboard XML itself. I prefer this way because when looking at a Github pull request, it’s easy to just Command-F on the page for those two terms.

An alternate way, open the storyboard in Interface Builder and look through each scene for the red or yellow warning dot that something isn’t right with Auto Layout.

Runtime Errors

Don’t forget that even if the storyboard itself looks okay, things can go wrong at runtime, especially if you are either programmatically tweaking constraints, or even must manually manipulating frames or views. If you’ve used Auto Layout long enough, you’ve definitely seen an error like this in the Console:

2012-07-26 01:58:18.621 Rolo[33597:11303] Unable to simultaneously satisfy constraints.
    Probably at least one of the constraints in the following list is one you don't want. Try this: (1) look at each constraint and try to figure out which you don't expect; (2) find the code that added the unwanted constraint or constraints and fix it. (Note: If you're seeing NSAutoresizingMaskLayoutConstraints that you don't understand, refer to the documentation for the UIView property translatesAutoresizingMaskIntoConstraints) 
(
    "<NSAutoresizingMaskLayoutConstraint:0x887d630 h=--& v=--& V:[UIButtonLabel:0x886ed80(19)]>",
    "<NSAutoresizingMaskLayoutConstraint:0x887d5f0 h=--& v=--& UIButtonLabel:0x886ed80.midY == + 37.5>",
    "<NSAutoresizingMaskLayoutConstraint:0x887b4b0 h=--& v=--& V:[UIButtonLabel:0x72bb9b0(19)]>",
    "<NSAutoresizingMaskLayoutConstraint:0x887b470 h=--& v=--& UIButtonLabel:0x72bb9b0.midY == - 0.5>",
    "<NSLayoutConstraint:0x72bf860 V:[UILabel:0x72bf7c0(17)]>",
    "<NSLayoutConstraint:0x72c2430 UILabel:0x72bfad0.top == UILabel:0x72bf7c0.top>",
    "<NSLayoutConstraint:0x72c2370 UILabel:0x72c0270.top == UILabel:0x72bfad0.top>",
    "<NSLayoutConstraint:0x72c22b0 V:[UILabel:0x72bf7c0]-(NSSpace(8))-[UIButton:0x886efe0]>",
    "<NSLayoutConstraint:0x72c15b0 V:[UILabel:0x72c0270]-(NSSpace(8))-[UIRoundedRectButton:0x72bbc10]>",
    "<NSLayoutConstraint:0x72c1570 UIRoundedRectButton:0x72bbc10.baseline == UIRoundedRectButton:0x7571170.baseline>",
    "<NSLayoutConstraint:0x72c21f0 UIRoundedRectButton:0x7571170.top == UIButton:0x886efe0.top>"
)

Will attempt to recover by breaking constraint 
<NSLayoutConstraint:0x72bf860 V:[UILabel:0x72bf7c0(17)]>

Break on objc_exception_throw to catch this in the debugger.
The methods in the UIConstraintBasedLayoutDebugging category on UIView listed in <UIKit/UIView.h> may also be helpful.

It’s nearly impossible to manually go through every view in an app with each storyboard code review, but that doesn’t mean you shouldn’t at least run the app and inspect any views that were added or changed with the commit. And the smaller the commit, the more focused this review can be. When going through this manual review of the app, keep an eye on the Console for that error message. It indicates something isn’t quite right with the Auto Layout, and probably needs to be addressed by the author.

Check For Valid IBActions

The last thing any of us want is our apps to crash. That’s even worse than a crappy user interface when it comes to losing a user’s faith in your app. Your app will crash if a UIControl is connected to an IBAction that no longer exists. You should look for this when doing storyboard code review. I’ve had this happen to me so many times, that having my peers look for this has saved my bacon more times than not. Specifically, this happens when:

1. Connect an IBAction to a UIControl in Interface Builder
2. Delete the IBAction method from the code.

Interface Builder does not disconnect the IBAction. If the end user triggers this action, the app will crash.

There are a couple ways to look for this: manually check every new or modified IBAction to ensure that the corresponding method exists, or manually test the app to verify than any new or modified control works. You can also do this verification through automated acceptance tests as well.

Open the Storyboard in Interface Builder

As part of storyboard code review, I recommend simply opening the storyboard in Interface Builder. This confirms two things:

1. The file can be opened. Don’t dismiss this. I often have had manual merges of storyboards go wrong such that the resulting file couldn’t even be opened in Interface Builder. Code shouldn’t get merged like this.
2. Interface Builder shouldn’t change the storyboard, just by opening it. Sometimes, I’ve observed that if the storyboard is using a custom font, and that font isn’t installed correctly on each machine, that Interface Builder will reposition all UILabels in the storyboard automatically when the file is opened. You can easily tell if this has happened because simply opening the storyboard will cause a local change in the file. This should not happen.

Look For Relevant Tests

This one is a little more subjective, and depends on your team’s agreed upon strategy for tests. On my team, we try to do our best to have redundantly high test coverage for our apps, at both the unit and acceptance test tiers of the testing pyramid. The acceptance tests are a little more loosely covered. During storyboard code review, I’ll check whether any of the changes made have corresponding tests written of the appropriate nature, and where possible make suggestions for better verification and coverage.

Wrap Up

Storyboards are code too. Just because you aren’t writing it by hand, doesn’t mean it shouldn’t be held to the same high standard as Swift or Objective-C code. A user’s opinion of the app can immediately drop if something in the user interface doesn’t look right. That starts in Interface Builder, hold yourself and your team to a higher standard. I’d love to hear how these suggestions for storyboard code review work for you.

Happy cleaning.