5
\$\begingroup\$

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
        }
    }
}

\$\endgroup\$
4
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Thanks for you clear and detailed answer Rob ! I will follow what you suggest, that was my primary implementation of the result block but I'm using it 3 times in the function so I was wondering if I could shorten my guard statements.But this it doesn’t entangle Error types and Result types made it. For the [Any] part, I am not using them in real life, I'm using real objects, but thanks for your concern ! \$\endgroup\$ – Nicolas Charvoz May 23 at 7:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.