Search

Refactoring to Clarity in Swift

Josh Justice

8 min read

Jan 22, 2017

iOS

Refactoring to Clarity in Swift

The first point of the Swift API Design Guidelines is that “Clarity at the point of use is your most important goal.” What does it look like to put that goal into practice? I found an answer recently while writing some integration tests. My test code started out difficult to follow, but by working on its clarity one step at a time, I ended up with an API I love. (And it turns out it’s exactly the same as an API offered by the Nimble library! I knew it was too good for me to be the first one to think of it!)

Let’s take a look at the process I went through to see what it looks like to improve the clarity of your code.

Failing to Fail

Consider a service object that connects to a web service to retrieve metadata about videos. One method on it retrieves all videos for a user with the given user ID. I want an integration test that takes a known user ID, hits the staging server, and checks some basic fields on the returned data. This is a good start:

var service = VideoService.serviceForEnvironment(.staging)

func testSuccessfulVideoRequestReturnsVideos() {
    service.videos(forUserID: "josh") { videos in
        XCTAssertEqual(videos.count, 3)
        XCTAssertEqual(videos[0].title, "Ponies")
    }
}

This test assumes the staging server has three videos for user “josh”, the first of which has the title “Ponies”. (A reasonable assumption if you know me!) When I wrote this test, I ran it before I set up that test data on the staging server to make sure it failed. But it succeeded! How could that be?

This happens because service.videos(forUserID:) makes a web request using the asynchronous URLSession.dataTask(with:), and it doesn’t call the passed-in closure until after that asynchronous web request returns. If we hide everything asynchronous from our test method, we have:

func testSuccessfulVideoRequestReturnsVideos() {
    service.videos(forUserID: "josh") {}
}

There are no assertions left! This explains why the test isn’t failing. After the asynchronous request is made, Xcode moves on from the test without waiting for it to return. If the entire test suite completes before the asynchronous response is received, Xcode will never check the assertions, so it will consider the test to have passed.

Asynchrony is great, but in this test we don’t want it. We want our test method to wait until the callback is called and all assertions are checked before it returns. How can we make this happen?

Getting It Working

Xcode provides a solution in the form of XCTestExpectations. They work like this:

  1. We create an expectation object.
  2. We fire off our asynchronous request, then we call waitForExpectations(timeout:) to wait for the expectation to be “fulfilled.”
  3. Inside our asynchronous callback we call fulfill() on the expectation when we’re done.

So let’s do those things I just said!

 func testSuccessfulVideoRequestReturnsVideos() {
+    let myExpectation = expectation(description: "service callback complete")
     service.videos(forUserID: "josh") { videos in
         XCTAssertEqual(videos.count, 3)
         XCTAssertEqual(videos[0].title, "Ponies")
+        myExpectation.fulfill()
     }
+    waitForExpectations(timeout: 10)
 }

The goal of adding this code was to get my test to fail because the test data wasn’t on the staging server yet. After I made these additions my test started failing, which is the correct behavior given that the test data was not yet set up. Then I added the test data to the staging server and the test passed.

Creating An Abstraction

So now the test works…but it’s far from clear. The number of lines in the test method almost doubled. And all the new lines deal with the mechanics of expectations, which are are irrelevant to my test logic. A reader of my test shouldn’t need to think through them.

All this is a sign that I need to create an abstraction; I need to hide these implementation details within a well-named method.

A clear API abstracts away details: it says what it does, not how it does it.

But how can I create a method to hide details if I have code I need to run before, during, and after my test code? Well, the “before” and “after” cases can be handled by the Execute Around Method pattern, which involves creating a function that wraps actions around a passed-in closure.

In this case we want our method to take care of creating and waiting for expectations, and call the passed-in closure to actually run the test:

+func waitForAsyncReturn(callback: (XCTestExpectation) -> Void) {
+    let myExpectation = expectation(description: "async call complete")
+    callback(myExpectation)
+    waitForExpectations(timeout: 10)
+}

 func testSuccessfulVideoRequestReturnsVideos() {
-    let myExpectation = expectation(description: "service callback complete")
+    waitForAsyncReturn { myExpectation in
         service.videos(forUserID: "josh") { videos in
             XCTAssertEqual(videos.count, 3)
             XCTAssertEqual(videos[0].title, "Ponies")
             myExpectation.fulfill()
         }
+    }
-    waitForExpectations(timeout: 10)
 }

(For clarity, I don’t show indentation changes in this diff, but the bulk of the test method got indented a level because it’s now inside the callback.)

The details of how to create and wait for expectations have been moved out of the test method and into a helper method. They’ve been replaced in the test method by a call to the waitForAsyncReturn helper method. It improves clarity at the point of use because its name states what it does without burdening the reader with implementation details.

Single Level of Abstraction

However, we haven’t made our point of use as clear as we can. My test still needs to know something about XCTestExpectations: it has to call fulfill() on the expectation passed to it. As currently written, waitForAsyncReturn hides some details but not all of them. It’s operating at different levels of abstraction, and this isn’t ideal.

A clear API operates at a single level of abstraction.

To keep our abstraction at a single level we need to avoid passing an expectation object into the callback. We can do this by wrapping the fulfill() call in a closure and passing that instead:

-func waitForAsyncReturn(callback: (XCTestExpectation) -> Void) {
+func waitForAsyncReturn(callback: (@escaping () -> Void) -> Void) {
     let myExpectation = expectation(description: "async call complete")
-    callback(myExpectation)
+    let expectationCallback = { myExpectation.fulfill() }
+    callback(expectationCallback)
     waitForExpectations(timeout: 10)
 }

 func testSuccessfulVideoRequestReturnsVideos() {
-    waitForAsyncReturn { myExpectation in
+    waitForAsyncReturn { callback in
         service.videos(forUserID: "josh") { videos in
             XCTAssertEqual(videos.count, 3)
             XCTAssertEqual(videos[0].title, "Ponies")
-            myExpectation.fulfill()
+            callback()
         }
     }
 }

The flow of control is a bit complex here, so let’s step through it:

  1. My test method calls waitForAsyncReturn and passes it a callback, which is the body of the test.
  2. waitForAsyncReturn calls the test body callback, and passes it another callback, which contains the expectation fulfillment.
  3. The test body executes the test, and calls the expectation callback when it’s done.
  4. The expectation callback calls myExpectation.fulfill()

Although that sequence is complex, the code at the point of use has gotten simpler. Now the details of XCTestExpectations are completely hidden.

Role-Based Names

Now we’ve gotten a consistent level of abstraction, but all these callbacks are making our clarity worse. Why am I calling back? When should I do it? What happens when I do it? We need to improve the names of these callbacks, and the way to do it comes directly from the API Design Guidelines:

“Name variables, parameters, and associated types according to their roles, rather than their type constraints.”

The callbacks are currently named for their type constraint instead of their roles. Yes, they’re things that should be called, but what happens when each is called? The role of the first is to start an asynchronous process, and the second is to signal that that process is done. So let’s name them asyncProcess and done to reflect this:

-func waitForAsyncReturn(callback: (@escaping () -> Void) -> Void) {
+func waitForAsyncReturn(asyncProcess: (_ done: @escaping () -> Void) -> Void) {
     let myExpectation = expectation(description: "async call complete")
-    let expectationCallback = { myExpectation.fulfill() }
-    callback(expectationCallback)
+    let done = { myExpectation.fulfill() }
+    asyncProcess(done)
     waitForExpectations(timeout: 10)
 }

 func testSuccessfulVideoRequestReturnsVideos() {
-    waitForAsyncReturn { callback in
+    waitForAsyncReturn { done in
         service.videos(forUserID: "josh") { videos in
             XCTAssertEqual(videos.count, 3)
             XCTAssertEqual(videos[0].title, "Ponies")
-            callback()
+            done()
         }
     }
 }

Now the difference between the callbacks is clearer. An asyncProcess is passed to waitForAsyncReturn. A done callback is created and passed to the asyncProcess. At some point the asyncProcess should call done().

This improves clarity at the point of use, too. You call waitForAsyncReturn() to wait for the passed-in asynchronous process to return, and you call done() when it’s done.

Coining a Phrase

This code still doesn’t read quite as nicely as the best Swift code, though. The API Design Guidelines indicate why:

“Prefer method and function names that make use sites form grammatical English phrases.”

Take a look at the method invocation as it currently stands:

waitForAsyncReturn { done in
    …
}

“Wait for async return done.” It sounds like a program, not English. How can we simplify it? In this case we can apply another principle from the API Design Guidelines: “omit needless words.” Do we have any needless words here?

  1. The word Return in the method name repeats what the done closure parameter already communicates.
  2. Async repeats what the asyncProcess parameter communicates. While that parameter name doesn’t show up at the point of use, it does show up in autocomplete when writing the test method. And it’s not really necessary to point out to the reader of the test that the block is asynchronous. The main thing is to communicate that we’re waiting until the code in the closure is done.

So let’s take omit both of those needless words from the method name. While we’re at it, we’ll change For to Until to make it read a bit better too:

-func waitForAsyncReturn(asyncProcess: (_ done: @escaping () -> Void) -> Void) {
+func waitUntil(asyncProcess: (_ done: @escaping () -> Void) -> Void) {
     let myExpectation = expectation(description: "async call complete")
     let done = { myExpectation.fulfill() }
     asyncProcess(done)
     waitForExpectations(timeout: 10)
 }

 func testSuccessfulVideoRequestReturnsVideos() {
-    waitForAsyncReturn { done in
+    waitUntil { done in
         service.videos(forUserID: "josh") { videos in
             XCTAssertEqual(videos.count, 3)
             XCTAssertEqual(videos[0].title, "Ponies")
             done()
         }
     }
 }

Now at the point of use the method reads waitUntil { done … }. This is the grammatical English phrase that we were shooting for. Its meaning is clear at a glance: “I’m waiting until I’m done, and here, at the end of this block, I’m done()”.

And although the main focus is clarity at the point of use, we’ve also managed to improve clarity at the point of declaration as well:

func waitUntil(asyncProcess: (_ done: @escaping () -> Void) -> Void)

If you ignore the required @escaping and Voids, this declaration simply says “wait until this asynchronous process is done.”

Conclusion

Swift is the most enjoyable when you’re working with clear APIs. Not only do they make code a delight to read, they make understanding and maintaining it a breeze. We’ve seen a few different principles for making clear APIs:

  1. A clear API abstracts away details: it says what it does, not how it does it.
  2. A clear API operates at a single level of abstraction.
  3. A clear API has parameters named for their roles, not their type constraints.
  4. A clear API forms grammatical English phrases.

You don’t need to get there all in one step. Read through the Swift API Design Guidelines and try applying the principles one at a time. You’ll be surprised with the clarity of the code you end up with!

Josh Justice

Author Big Nerd Ranch

Josh Justice has worked as a developer since 2004 across backend, frontend, and native mobile platforms. Josh values creating maintainable systems via testing, refactoring, and evolutionary design, and mentoring others to do the same. He currently serves as the Web Platform Lead at Big Nerd Ranch.

Speak with a Nerd

Schedule a call today! Our team of Nerds are ready to help

Let's Talk

Related Posts

We are ready to discuss your needs.

Not applicable? Click here to schedule a call.

Stay in Touch WITH Big Nerd Ranch News