Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using init(attempt:) always produces Result with AnyError type #255

Closed
philtre opened this issue Nov 11, 2017 · 6 comments
Closed

Using init(attempt:) always produces Result with AnyError type #255

philtre opened this issue Nov 11, 2017 · 6 comments

Comments

@philtre
Copy link

philtre commented Nov 11, 2017

I'm using Result in one of my projects but the thing that I would really appreciate is the ability to initialize a Result with a throwing closure but with an arbitrary Error type. Eg.:

let result = Result<String, WrapperError>(attempt: { throw SomeOtherError() })

where WrapperError can wrap other Errors just like AnyError

I've submitted a pull request here: #254

@mdiep
Copy link
Contributor

mdiep commented Nov 21, 2017

Why are you using WrapperError instead of AnyError?

I'm curious why you'd do this and what benefit you're getting from it.

@philtre
Copy link
Author

philtre commented Nov 23, 2017

So, WrapperError is just an example of a concrete struct or enum that conforms to Error and can be initialized with an arbitrary Error (just like AnyError).

Let's say I have an error like this:

enum StringMangleError : Swift.Error, ErrorInitializing {
    case emptyString
    case stringTooLong
    case other(Error)
    
    init(_ error: Swift.Error) {
        if let stringMangleError = error as? StringMangleError {
            self = stringMangleError
        } else {
            self = .somethingOtherHappened(error)
        }
    }
}

And I have a function that throws:

func mangle(_ string: String) throws -> String {
    guard !string.isEmpty else { throw StringMangleError.stringEmpty }
    guard string.count < 256 else { throw StringMangleError.stringTooLong }
    let mangledString = //... do some mangling
    return mangledString
}

Now let's say I want to do the mangling asynchronously. The only way I can think of writing that function now would be:

func asyncMangle(_ string: String, completion: (Result<String, StringMangleError>) -> Void) {
    do {
        completion(.success(try mangle(string)))
    } catch {
        completion(.failure(StringMangleError(error)))
    }
}

But with the proposed changes, it would be as easy as:

func asyncMangle(_ string: String, completion: (Result<String, StringMangleError>) -> Void) {
    completion(Result(attempt: { return try mangle(string) }))
}

@mdiep
Copy link
Contributor

mdiep commented Nov 26, 2017

I think the way you'd normally do this with Result would be:

func asyncMangle(_ string: String, completion: (Result<String, StringMangleError>) -> Void) {
    let result = Result<String, AnyError>(attempt: { return try mangle(string) })
        .mapError { StringMangleError($0.error) }
    completion(result)
}

It's not as concise as your example, but I'm not sure the indirection is worth the brevity in this case.

@philtre
Copy link
Author

philtre commented Nov 29, 2017

Right. I guess that's a slightly more brief and functional way of writing that.

However, I wasn't aiming so much for brevity but rather better inference, especially when it comes to errors.

It might seem inconsequential from the provided example but in my current project I'm connecting many classes where each class has its own Error enum. A less cumbersome way to map between results where two Results have the same associated Value type but different Error types would be much appreciated. Better inference for attempt: initializers (without the need to go through AnyError) would also greatly reduce clutter in my codebase.

Currently, Result(attempt: {"foo"}) always produces Result<String, AnyError>, with no possibility to infer other Error wrappers.

The following will currently not build:
let result: Result<String, CustomError> = Result(attempt: {"foo"} )

To achieve this, one needs to manually map AnyError to WrapperError (as you suggested):
let result: Result<String, WrapperError> = Result(attempt: {"foo"} ).mapError{ WrapperError($0.error) }

Additionally, Result({ "foo"}) also currently produces Result<String, AnyError>, although a non-throwing closure should in my opinion actually produce a Result<String, NoError>.

There is also currently no default inference for Result(value: "foo") although the inferred type could easily be Result<String, NoError>

In my updated pull request, the following is true
(assuming BasicError is just a simple struct that conforms to Error):

  • Result({"foo"}) is equivalent to Result<String, NoError>.success("foo")
  • Result(attempt: {"foo"}) is equivalent to Result<String, AnyError>.success("foo")
  • Result(attempt: { throw BasicError() } is equivalent to Result<String, AnyError>.failure(AnyError(BasicError()))
  • let result: Result<String, WrapperError> = Result(attempt: {"foo"}) is equivalent to Result<String, WrapperError>.success("foo") and will build if WrapperError conforms to ErrorInitializing
  • let result: Result<String, WrapperError> = Result(attempt: { throw BasicError()} ) will build and is equivalent to Result<String, WrapperError>.failure(WrapperError(BasicError())) if WrapperError conforms to ErrorInitializing
  • Result(attempt: { throw WrapperError(BasicError())} ) is equivalent to Result<String, AnyError>.failure(AnyError(BasicError())) if WrapperError conforms to ErrorConvertible (AnyError will actually wrap WrapperError's underlying Error)
  • let basicResult = Result<String, BasicError>(value: "foo") can be error-mapped with basicResult.mapError(to: WrapperError.self) or simply let wrappedResult: Result<String, WrapperError> = basicResult.mapError() if WrapperError conforms to ErrorInitializing

@mdiep
Copy link
Contributor

mdiep commented Dec 3, 2017

Here's what I'd suggest:

  • Result(attempt:) should only be used when calling code you don't control. Anything you do control should return a Result. You should wrap external code with an interface that returns a Result.

  • The custom errors that your code returns can be mapped into a different cases of an outer enum if you need to handle them in a single spot.

In general, relying on inference isn't a great idea because Swift's type inference is limited. So I'm always hesitant to add an API that relies solely on inference.

@ikesyo
Copy link
Member

ikesyo commented Apr 10, 2018

Closing this due to inactivity.

@ikesyo ikesyo closed this as completed Apr 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants