Enum for handling network request errors in Swift

Posted on

Problem

I’m working on a new API written in Swift 5 and I wanted to play with the new Result. I wanted to know what you guys think about this syntax:

enum NetworkRequestError: Error {
    case hostNotAvailable
    case accountNotAvailable

    func finish<T>() -> Result<T, NetworkRequestError> {
        return .failure(self)
    }
}

public class NetworkClient: TestableAPI {
    var host: String?

    func fetch(result: @escaping (Result<(any: [Any], any1: [Any]), NetworkRequestError>) -> Void) {
        guard host != nil else {
            result(NetworkRequestError.hostNotAvailable.finish())
            return
        }
    }
}

Solution

I would not recommend this pattern. I see no compelling reason to encumber NetworkRequestError (and presumably every other Error enumeration throughout your project) with this finish cruft.

So, instead of:

enum NetworkRequestError: Error {
    case hostNotAvailable
    case accountNotAvailable

    func finish<T>() -> Result<T, NetworkRequestError> {
        return .failure(self)
    }
}

func fetch(result: @escaping (Result<(any: [Any], any1: [Any]), NetworkRequestError>) -> Void) {
    guard host != nil else {
        result(NetworkRequestError.hostNotAvailable.finish())
        return
    }

    ...
}

I’d instead suggest:

enum NetworkRequestError: Error {
    case hostNotAvailable
    case accountNotAvailable
}

func fetch(result: @escaping (Result<(any: [Any], any1: [Any]), NetworkRequestError>) -> Void) {
    guard let host = host else {
        result(.failure(.hostNotAvailable))
        return
    }

    ...
}

This is more concise, conforms to established patterns (making it easier to reason about) and it doesn’t entangle Error types and Result types.

Also you’re undoubtedly checking host because you’re going to use it later in this method, so you might as well do guard let, like above. It saves you from having to do subsequent unwrapping of the optional.


I’m assuming your question was primarily about the Error type and this finish method. I must say that I am equally uneasy about the .success associated type, namely the tuple of [Any] arrays. Perhaps this was just a placeholder in your example, but I generally find references to Any to be code smell. Often specific types or generics would be better than Any to types in one’s code. If you have a question about why you’re using a “success” type which is two arrays of Any, that might warrant its own question, showing why you did that. It, too, could probably be improved.

Leave a Reply

Your email address will not be published. Required fields are marked *