objc.io

00:06 Today we're starting a new series in which we show some techniques for refactoring large view controllers.

We'll work with a view controller from the open source codebase of the Wikipedia iOS app. The changes we make aren't meant as criticisms. On the contrary: it's because the code is written very well that it's pretty easy for us to understand and use for demonstration purposes in this series.

Wikipedia's Places View Controller

01:33 The view controller we chose to examine is the PlacesViewController, which shows up in the Places tab. It has a map view showing Wikipedia entries as map annotations. When we select an annotation, an overlay appears with a short description and some buttons. There's also a search box with filter options and a list of everything currently on the map. When we type in the search box, it gives us both autocomplete suggestions and our search history. Clearly, there are a lot of features on this screen, and most of them are written in a single view controller using around 2,700 lines of code.

02:59 There can be many reasons to refactor code. We do so because we want to be able to understand the code better, and in general, a smaller view controller is easier to understand. The PlacesViewController has a lot of state, and any of its methods could potentially access that state. This means one has to be very careful when changing a method and make sure to not create any conflicts with other parts of the view controller.

By pulling methods out into separate, logical components or top-level functions, we will hopefully make it easier to see what the view controller is really about. And in doing so, we'll also make the individual components easier to understand.

Pulling Out a Helper Function

04:13 We start with some methods that are easiest to pull out into helper functions. The first method takes two coordinate regions and checks if there's a certain distance between them:

func isDistanceSignificant(betweenRegion searchRegion: MKCoordinateRegion, andRegion visibleRegion: MKCoordinateRegion) -> Bool {
    // ...
}

05:11 By reading the signature, we suspect it can operate independently of everything else in the view controller. But because it exists inside the view controller, we can't be sure that it doesn't rely on other properties. We test this by moving the method into a new file, making it a top-level function. The project still builds after doing so, which confirms our suspicion: the function works without any access to the view controller's state.

06:58 Why does it make sense to remove the function from the view controller? We could argue that it doesn't matter where it lives if it doesn't add any state to the view controller. But the downside of having the function inside the view controller is that we're not sure whether or not it messes with the state.

By moving the function to a separate namespace, we isolate it and we can see exactly which parameters it depends on and what comes out of it. In other words, it's now a pure function because it doesn't have any side effects. Therefore, it's not only easy to understand, but also easy to test.

08:10 To follow a more Swift-like pattern, we put the function in an extension of the type it operates on:

import Foundation
import MapKit

extension MKCoordinateRegion {
    func isDistanceSignificant(to visibleRegion: MKCoordinateRegion) -> Bool {
        // ...
    }
}

09:01 Inside the function, we can now use self anywhere the searchRegion parameter was used. Next, we update the method calls in the view controller:

let movedSignificantly = regionThatFits.isDistanceSignificant(to: visibleRegion)

10:28 Because the function we pulled out is a pure function, we feel comfortable with this refactoring, and we don't need to write any tests for it. And by pulling out this kind of helper code — even if it's specific to just this view controller — we make the code easier to read and process.

Pulling Out Layout Code

11:12 The second example of helper code to pull out is a bit more complicated. When a map annotation is selected, a popover shows up next to it. The position for the popover is calculated based on the superview's size, the search box overlay, the status bar, and other elements. This calculation is done in the view controller method adjustLayout, which does a whole lot of frame math and finally updates the popover's frame. The method depends on a couple of properties:

func adjustLayout(ofPopover articleVC: ArticlePopoverViewController, withSize popoverSize: CGSize, viewSize: CGSize, forAnnotationView annotationView: MapAnnotationView) {
    // ...
    articleVC.view.frame = CGRect(origin: CGPoint(x: x, y: y), size: popoverSize)
}

12:19 The method adjustLayout takes the popover controller and manipulates its view's frame. We want to separate the calculation and the setting of the frame, and in the end, we want to pull out the computation part. But first, we split up the method into two methods side by side:

func adjustLayout(ofPopover articleVC: ArticlePopoverViewController, withSize popoverSize: CGSize, viewSize: CGSize, forAnnotationView annotationView: MapAnnotationView) {
    articleVC.view.frame = computeLayout(for: popoverSize, viewSize: viewSize, forAnnotationView: annotationView)
}

func computeLayout(for popoverSize: CGSize, viewSize: CGSize, forAnnotationView annotationView: MapAnnotationView) -> CGRect {
    // ...
    return CGRect(origin: CGPoint(x: x, y: y), size: popoverSize)
}

14:49 That sets up our refactoring and it still compiles. Now we want to change the method to only take value types, so we have to do something about the MapAnnotationView parameter, which is only used for its frame and its center point. We replace the parameter directly with a frame, and we recompute the center point from the frame:

func adjustLayout(ofPopover articleVC: ArticlePopoverViewController, withSize popoverSize: CGSize, viewSize: CGSize, forAnnotationView annotationView: MapAnnotationView) {
    articleVC.view.frame = computeLayout(for: popoverSize, viewSize: viewSize, forAnnotationView: annotationView.frame)
}

func computeLayout(for popoverSize: CGSize, viewSize: CGSize, forAnnotationView annotationViewFrame: CGRect) -> CGRect {
    // ...

    let annotationSize = annotationViewFrame.size
    let annotationCenter = view.convert(annotationViewFrame.center, from: mapView)

    // ...
}

15:59 CGRect doesn't have a center property, so we add it in another helper file:

extension CGRect {
    var center: CGPoint {
        return CGPoint(x: midX, y: midY)
    }
}

16:56 Now we have a separate helper method that only takes value types, but we don't yet know what kind of dependencies are hiding in the body of computeLayout. We find out by moving the method out of the view controller and into the helper file. The compiler shows errors for all properties that the method can no longer access.

17:23 The first error that pops up is a missing enum type, which actually belongs to the method only, so we move it into the helper file as well:

enum PopoverLocation {
    case top
    case bottom
    case left
    case right
}

17:52 The next error we get is for accessing the view controller's view. The view is used to convert the annotation's center point from the map view's coordinate system:

let annotationCenter = view.convert(annotationViewFrame.center, from: mapView)

We don't have access to the view in our helper function — nor do we want to have access — so we decide that the passed-in frame should already be converted to the view controller's view. This allows us to replace the line above with the following:

let annotationCenter = annotationViewFrame.center

18:51 And we do the conversion in the adjustLayout method of the view controller:

func adjustLayout(ofPopover articleVC: ArticlePopoverViewController, withSize popoverSize: CGSize, viewSize: CGSize, forAnnotationView annotationView: MapAnnotationView) {
    let annotationFrame = view.convert(annotationView.frame, from: mapView)
    articleVC.view.frame = computeLayout(for: popoverSize, viewSize: viewSize, forAnnotationView: annotationFrame)
}

19:27 The next missing dependency is the property isViewModeOverlay. It indicates whether or not the search box overlay is there, which is the case on iPad but not on iPhone. If the search box overlay, listAndSearchOverlayContainerView, is present, we also need its frame. We add an optional parameter for the frame so that we can pass in nil if the overlay isn't present. By taking the frame and not the view, we're still only depending on value types:

func computeLayout(for popoverSize: CGSize, viewSize: CGSize, forAnnotationView annotationViewFrame: CGRect, overlayFrame: CGRect?) -> CGRect {
    // ...
    if let listAndSearchOverlayContainerViewFrame = overlayFrame {
        // ...
    }
    // ...
}

21:10 From the view controller, we pass in either the overlay's frame or nil, depending on the Boolean property isViewModeOverlay:

func adjustLayout(ofPopover articleVC: ArticlePopoverViewController, withSize popoverSize: CGSize, viewSize: CGSize, forAnnotationView annotationView: MapAnnotationView) {
    let annotationFrame = view.convert(annotationView.frame, from: mapView)
    let overlayFrame = isViewModeOverlay ? listAndSearchOverlayContainerView.frame : nil
    articleVC.view.frame = computeLayout(for: popoverSize, viewSize: viewSize, forAnnotationView: annotationFrame, overlayFrame: overlayFrame)
}

21:44 There are two errors left. The first one is caused by accessing the view to calculate its center point:

let viewCenter = CGPoint(x: view.bounds.midX, y: view.bounds.midY)

Since we already know the view's size, we can recreate the view's bounds and use the computed property we wrote earlier to get the center point:

let viewCenter = CGRect(origin: .zero, size: viewSize).center

22:57 The last error is about the missing property extendNavBarView:

let navBarHeight = extendedNavBarView.frame.height

We need the height of the navigation bar, which depends on the device and its orientation, so we pass this in as another parameter:

func computeLayout(for popoverSize: CGSize, viewSize: CGSize, forAnnotationView annotationViewFrame: CGRect, extendedNavBarHeight: CGFloat, overlayFrame: CGRect?) -> CGRect {
    // ...
    let navBarHeight = extendedNavBarHeight
    // ...
}

23:42 After updating the call site, the code compiles again:

func adjustLayout(ofPopover articleVC: ArticlePopoverViewController, withSize popoverSize: CGSize, viewSize: CGSize, forAnnotationView annotationView: MapAnnotationView) {
    // ...
    articleVC.view.frame = computeLayout(for: popoverSize, viewSize: viewSize, forAnnotationView: annotationFrame, extendedNavBarHeight: extendedNavBarView.frame.height, overlayFrame: overlayFrame)
}

Discussion

23:59 These changes were pretty straightforward. We didn't change any of the logic; we changed just the parameters that are passed around. From an object-oriented standpoint, one could argue we made things more complex by pulling out specific logic and having to pass a long list of parameters instead of just accessing the view controller's properties. But on the other hand, we now know for sure that the code we pulled out is only dependent on the parameters we pass in.

24:48 Another advantage of our refactoring is that the computeLayout function only uses value types, which means it's really easy to test: we can simply pass in arbitrary values and check the result we get back.

25:29 Today's examples were the simplest changes we could make to the codebase. And there are more cases like these which can be dealt with in a way that's similar to what we showed, so we won't have to cover those here.

But we will dive into refactoring other, more interesting parts: we'll pull some code out into the model layer, pull other code out into a service class, and maybe even split the whole thing up into child view controllers. We look forward to doing all of this in upcoming episodes!