Recognising self-improvement

When I started working on Via a couple of years back I was pretty new to the iOS development experience. It was a hobby project and not intended for others, rather I just wanted to put something together for my own somewhat niched travelling needs. As the project grew, however, so did my aspirations.

Not financially, mind. What I wanted was a richer feature set, but with my very limited knowledge I really didn’t know what standards/patterns/frameworks to use, so I sort of came up with my own homemade solutions — one more convoluted than the other — as I went.

In the end, Via turned out quite nice on the outside, but the inside was still much of a mess. Now, roughly one year after its initial release on App Store, most of the app has been rewritten through all the accumulated experience since last iteration. While still no masterpiece, it does very clearly measure the progress I’ve made as a developer, both on iOS and otherwise.

Let’s have a look at some code responsible for fetching/retrying departures from Västtrafik’s API. The code below has been annotated, streamlined and trimmed, while the original code is 40+ lines.

func fetch() { error in
    fetchDepartures() { error in
        if error == .cancelled { return }

        // Fetch ok
        if error ==  .none {

        // Auth error
        } else if error == .accessToken {
            fetchAccessToken() { error in

                // Auth ok
                if error == .none {
                    fetchDepartures() { error in
                        if error != .none {}
                    }

                // Auth error
                } else {}
            }

        // Fetch error
        } else {
            fetchDepartures() { error in
                if error != .none {}
            }
        }
    }
}

Now, that’s a hot mess. Callback hell with a hefty amount of if-else nesting added for good measure. The point was clearly to handle request errors and retry different requests depending on the error. The updated code looks something like this:

func retryDelayed() -> Observable<Response> {
    return retry(.delayed(maxCount: 2, time: 2), shouldRetry: { error in
        // Condition for retrying.
    })
}

func retryWithAuth() -> Observable<Response> {
    return retryWhen { $0.flatMap { error -> Observable<AccessToken> in
        if self.responseStatusCode(from: error) == 401 {
            // Do something for auth errors.
        } else {
            // Do something else for other errors.
        }
    }}
}

Much better! Not only has the error handling been generalised, since it’s part of an Rx data stream, it can be injected where needed:

func parseApiResponse() -> Observable<Any> {
    return asObservable()
        .filterSuccessfulStatusCodes()
        .retryWithAuth()
        .retryDelayed()
        .catchError { return Observable.error(error) }
        .mapJSON()
}

Now, although the fetch/retry logic was indeed bad, you could still somewhat make out what was happening and why even if you were new to the code. Let’s have a look at something even worse, and pretty much impossible to understand without heavy annotation. As with the previous example, I have cut quite a lot and made it readable below. The original beast was a whopping 140+ (!!) lines!

func fetchDepartures(completion: () -> ()) {
    let header = ["Authorization": "Bearer \(accessToken)"]
    var body = [String: String]()

    var bodies = [[String: String]]()

    body["timeSpan"] = "5"
    bodies.append(body)

    body["timeSpan"] = "90"
    bodies.append(body)

    for i in 0 ..< bodies.count {
        makeApiRequestMulti() { data in
            guard let data = data, let result = getResultFrom(data: data) else {
                // Complex error and completed/incomplete requests handling.
                completion()
                return
            }

            guard let departureList = result["DepartureBoard"] else {
                // Complex error and completed/incomplete requests handling.
                completion()
                return
            }

            if let departureData = departureList["Departure"] {
                // Successful data handling.
            }

            // Additional checks for completed/incomplete requests.
            completion()
        }
    }
}

So, we have multiple requests being made, one for each body, differentiated by the time span of the desired departures. What’s not visible here is the complex handling of failed requests, partial buffering of departures and what to return and when depending on those. It’s all homemade glorious jank, and something that haunted me throughout most of my time with the (old) app. In the new app, a framework called Moya, along with Rx extensions for it, puts all of this behind abstraction through a neat and clean enum structure:

var headers: [String: String]? {
    switch self {
    case .departures:
        return ["Authorization": "Bearer \(accessToken)"]
}

var parameters: [String: Any] {
    switch self {
    case .departures(let date, let timeSpan, let origin):
        return [:]
}

var task: Task {
    return .requestParameters(parameters: parameters)
}

The actual request call is then handled by another class using pure RxSwift:

func getDepartures(date: Date, origin: Stop) -> Observable<[Departure]> {
    return Observable
        .merge([
            _getDepartures(date: date, timeSpan: 5, origin: origin),
            _getDepartures(date: date.inFuture(mins: 5), timeSpan: 90, origin: origin)
        ])
}

private func _getDepartures(date: Date, timeSpan: Int, origin: Stop) -> Observable<[Departure]> {
    return VasttrafikProvider.shared.rx
        .request(.departures(date: date, timeSpan: timeSpan, origin: origin))
        .parseApiResponse()
        .mapArray(type: Departure.self)
}

Granted, it’s easy to look back and laugh at silly design choices, but it’s not that didn’t understand that the original implementation was bad. I knew what I wanted to achieve, but I couldn’t turn those ideas into efficient and maintainable code. I had to settle for making it work, and, well, it did. The new code uses network frameworks, Rx patterns, Swift extensions and overall improvement of what does what, and where. This is surely no coincidence, as it’s quite evident that I’ve learned some very specific lessons on networking, error handling and the dark, dark side of state machines.

Working professionally, there’s usually very little time for revisiting old projects or code bases unless they’re downright broken. And to be honest, most of the time it’s not exactly thrilling either. Thus, going back to rewrite Via wasn’t originally something I was very excited for, but that gradually changed the more I got new code in place. I didn’t care too much about old mistakes, rather I was very happy to see how much more I knew now than before.

So, once in a while, if you’re feeling under the pressure to keep up with the latest in tech, take a look back and appreciate your progress. In your spare time it’s ok to learn things the hard way. You can dabble all you want, try out new things, get things done, just not care too much if it’s good or not. Which, in my opinion, is a seedbed for improvement. And if you’re brave enough, share it and let others reflect on it, just like I did.