SE-0235 - Add Result to the Standard Library

Evolution Proposal Reviews
1

The review of SE-0235 - Add Result to the Standard Library begins now and runs through November 12, 2018.

Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to the review manager (via direct message in the Swift forums).

What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

  • What is your evaluation of the proposal?
  • Is the problem being addressed significant enough to warrant a change to Swift?
  • Does this proposal fit well with the feel and direction of Swift?
  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks!
Chris Lattner
Review Manager

33 Likes
2

I think this is a worthy addition to the standard library, but I do have some concerns about the proposal:

  • Including the catamorphism (fold) raises some design questions I don't think we need to deal with right now.
  • Shadowing Error seems like a bad idea.
  • The value vs. success naming divide is a little weird.
  • unwrapped() is a useful operation, but nothing else in the type's interface talks about the value being "wrapped", so the name is weird.
  • Overloading flatMap by the throwing-ness of the argument closure is a really bad idea.

I don't mind Result having two type arguments, though.

So I'm in favor of the basic idea, but there are a lot of naming issues that I think should've come to a better resolution before this entered review.

22 Likes
3

Since Error existentials have a single retainable pointer representation, we should consider making Error self-conforming. It does not have any constructors or static methods and the representation works out so it should be possible to change the type checker to allow this and then it will just work.

As a bonus, in Chris's implementation the extension Result where Self.Error == Error can be merged with extension Result where Self.Error : Error.

8 Likes
4

I am in favor of this proposal but would like see more discussion in the source compatibility section. I am not sure what happens to projects that already use such a type. Is this change source incompatible? Will they require migration? Another question: will corelibs types that have async APIs (like URLSession) adopt this pattern?

  • What is your evaluation of the proposal?

+1

The Result enum pattern has been quite useful in the Swift package manager project as well.

  • Is the problem being addressed significant enough to warrant a change to Swift?

Yes, especially for working with async APIs.

  • Does this proposal fit well with the feel and direction of Swift?

Yes.

  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

No.

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Skimmed the proposal.

5 Likes
5

Was about to write a quick note to similar effect but will instead simply say that I agree with @John_McCall’s critiques—except that “success(Value)” and “failure(Error)” seem fine to me, along the same lines as “some(Wrapped)” for Optional. However, having “.success”, “.value” and “.unwrapped” does seem inconsistent, and I wonder if all of these APIs are even necessary.

3 Likes
6

If someone is already using a Result type, there will be a collision. This can be mitigated in a variety of ways, most easily by renaming their implementation of Result to something unique and then migration to Swift's Result over time. I believe this is a first for Swift, so I'm not sure what more needs to be done to help users with such a transition. For what it's worth, I migrated SPM's Result implementation fairly quickly, not counting the many toolchain rebuilds that needed to happen to find all of the uses. Migrating Alamofire's was also easy, since internal use is limited.

As for core API usage, I think a follow up proposal for some sort of Obj-C markup would be required to allow mapping multipart completion handlers to a Result and then the Swift overlays updated. I think it makes sense, but can come at any point after the type is added.

2 Likes
7

If we did this, we would want to ensure that erasing T: Error to Error does not add an extra box when T is dynamically Error.

2 Likes
8

We already have to do this for NSError interop on Darwin, so we should be OK on that front.

4 Likes
9

Ah, right, of course.

10
  • What is your evaluation of the proposal?

Huge +1. I cannot imagine working on a project without a Result type. The design of the proposal is very similar to the Result type I use. It would benefit the community to standardize this type.

I would prefer some minor changes to the API, but support the core team in making the final decision about details.

Specifically:

  • For some reason I find success and failure case names to be confusing. I have worked with an error type using these names for years and still forget what they are when I need to write a switch over a result. I prefer value and error case names.

  • I think unwrap would be a better name than unwrapped. It may be possible to identify a name better than unwrap as well but I'm not sure what it would be.

  • I understand that fold is a common name used in FP but am not sure whether this name is the best fit for Swift. This method is easy to add in an extension and is easy to add later. I agree with @John_McCall that it may be best to leave it out for now.

  • Is the problem being addressed significant enough to warrant a change to Swift?

Yes. This is a foundational type that is used in every project I work on. Not having a standard type can cause compatibility issues between dependencies. It belongs in the standard library.

  • Does this proposal fit well with the feel and direction of Swift?

Very much.

  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

As mentioned, it is very similar to the Result types I have worked with.

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I participated in the pitch thread in depth.

4 Likes
11

Removing this is fine, I only added it since being able to operate on both sides of a Result simultaneously seemed useful.

A few of these naming issues came up during the pitch phase, and in an attempt to not rebikeshed the entire type, I defaulted to names commonly shared by community implementations. In particular, the success / failure case names and the Value / Error generic types were almost universally common. Personally, I think their names also stand to reason, to better indicate the semantic meaning of the type through the case names, and the separation of types through the type names. None of the other names proposed seemed better enough to justify diverging from the community implementation.

unwrapped() is a common feature, but is sometimes known as unwrap(), throw() or value(). I stuck with unwrapped() because of the possibility of it aligning with a common unwrapping feature in the future. If that's not of value, then I think a simple renaming is fine here.

As for the mapping operations, those were thoroughly discussed in the pitch. It was thought the throwing nature of the transform was similar to having another Result in the mix, and so flattening was the appropriate action. Obviously we want to support throwing transforms on the type, so what do you think the appropriate signature would be?

12

Shadowing is generally a bad idea, but I (and many others) have worked with Result types that use a generic parameter Error for years and have never encountered a problem. Is there evidence that this is a problem in practice? Usage has been pervasive enough that if it is I think there would be evidence of that by now.

2 Likes
13

FINALLY.

Yes, absolutely. The propagation of custom Result types in the open source community is the source of a bunch of frustration when including different packages requires distinct Result implementations

Yes. I was initially hesitant about making the Failure type not be an Error, but I find the URLSession example quite compelling (ie, having a tuple type be the failure type).

N/A

Lurked and read. I have my own implementation of Result in my personal frameworks that I would love to get rid of once this is adopted.

Some thoughts I would love to see addressed:

• I echo the concern about the spelling of the failure generic type as Error. I think that'll cause confusion as developers try to grok the distinction between Error and Swift.Error. Since the case is called .failure, how about naming the corresponding generic type to be Failure too?

• I don't like the inconsistency between the use of value and success in the API. The case is .success(_), but the property is var value: Success?. It should either be value, or it should be success. Let's not mix the two and let's keep the naming consistent.

• Similarly, there is inconsistency between the use of failure and error. The case is .failure, but the methods all refer to error. It should be one or the other: ie mapFailure and flatMapFailure.

• The inclusion of isSuccess: Bool is excellent, but I was surprised to see that isFailure: Bool was missing? Please include that for completeness. For when you have to do this, it is semantically much clearer to check for "is this a failure" instead of "is this not a success".

12 Likes
14

I’m very strongly in favor of this proposal, and I’d prefer to keep the naming as-is.

A lot of the criticisms of the naming fail to acknowledge that the names that have been chosen are not arbitrary — the Result type is an established standard within the Swift community and has been for several years.

The naming of the cases and methods are recognized terms-of-art for many Swift developers, and changing them would mean that instead of a near drop-in-replacement for the existing Result types used in most projects, developers would need to make unnecessary breaking changes to their APIs.

  • What is your evaluation of the proposal?

I agree with the design and rationale for the API as proposed.

  • Is the problem being addressed significant enough to warrant a change to Swift?

Very much so. I use some form of Result for basically every project I work on, and the fact that it can’t be conveniently exposed in public APIs without either adding a dependency on a 3rd party framework or requiring consumers to manually convert between different Result types with near-identical APIs is a constant frustration.

  • Does this proposal fit well with the feel and direction of Swift?

Yes. It is paving a well-trodden cowpath within the Swift development community.

  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

I’ve used several Swift libraries that implement Result and this proposal perfectly captures the best features of all of them.

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I’ve been following the proposal for several weeks.

4 Likes
15

+1.

enum Result<Wrapped, Failure> {
    case some(Wrapped)
    case error(Failure)
}

I like the above naming because I think it would be easier to replace code that uses the Optional enum to use Result instead. Naming would not be an issue for me if we had an assurance that the if let or ?? unwrapping behavior could be applied to Result in a later version of Swift.

I think the proposal really wants to push for success and failure but even in the mentioned languages there is not strong indication that that naming is 'word of art', (I am specially thinking about Rust's Ok and Err). I'd be more inclined with .ok and .err than .success and .failure.

Yes. Making this change now would allow this type to be baked into ABI would should make it easier to people to share code.

yes.

While I know most languages have a Result type in the standard library or as a package. I have not used a result type in any of my projects. The only time I was exposed to this type of error handling was in golang where it is a convention to return a tuple like (string, error). Ultimately I think that the need to have a result type in the library is because we need something that is able to travel between being an Optional and Error handling.

https://go.googlesource.com/proposal/+/master/design/go2draft-error-handling-overview.md#swift

I am a huge fan of introducing Result but only if the naming aligns with Optional so we could "upgrade" an Optional returning method to use a Result that happens to carry an Error when there is not valid result.

Participated in last thread and read some of the previous threads.

1 Like
16

Overall -1.

No. I've worked probably over a dozen of Swift codebases in a variety of domains and on a variety of platforms. I'm also a big fan of adding nice things to make your project. Yet I've never had an instance where I needed or wanted Result. I'm not exaggerating: zero times.

In day-to-day iOS programming, nearly all the errors that need to be handled are from asynchronous actions (naturally, it's the first example in the proposal) and all of those things are better handled with promises or futures. In that vein, I have written a Swift promise library and I use it in every iOS project I work on. Result has frankly not been useful enough to me to justify its addition in any project, much less the standard library, and its value pales in comparison to something like Promise.

I don't think so. The way this proposal is laid out, it adds a Result type without any of the thoughtful interoperability that went into the way that, for example, Swift 2's throws and defer worked together. This proposal includes an initializer that takes a throwing block, but no compiler-level integration. i.e., you might expect to be able to:

let stringResult = String(contentsOfFile: configuration) // (stringResult is a Result, with no `try` or initializer required)

Instead, you have to use the Result initializer. Using the Result { } initializer is like living without optional promotion and having to explicitly wrap everything in .some. Without Swifty stuff like that, this feature feels tacked on, mostly something to allow people to delete their own Result implementations from their projects.

I see the value in having incremental proposals, but I also think that Swift should be improved with granular changes that have merit on their own, rather than half-steps. If Swift's error system were originally designed with a nominal Result type in mind, I would be less opposed.

As mentioned earlier, I've mostly worked with promise libraries, since I've never had a need for a Result type, but I've worked with Promises extensively and they have a very similar shape. That experience is part of what makes me think this doesn't hold its value.

Quick reading.

25 Likes
17

This is a spot-on critique. It's one I've mentioned before in these discussions and it completely slipped my mind in my review. I absolutely believe we need better compiler integration with Result as well.

10 Likes
18

I think the success/failure spelling is the best of the various options. I agree with Dave about the naming inconsistencies. All instances of value in the various APIs should be changed to success.

I don't think the Error shadowing is a big deal, and it seems like the most appropriate name for that parameter, so I'm slightly in favor of keeping it as proposed.

The proposal as it stands has too much API bundled with it. I think we should get a bare bones Result in and any additional APIs could be follow up proposals.

public var value: Value? { get }
public var error: Error? { get }

These just make it so you can write if let rather than if case let. I don't think they are necessary.

public var isSuccess: Bool { get }

Why would one want to use a result as a Bool? Getting at the value inside the success case seems like a fundamental use of the type.

public func mapError<NewError>(
  _ transform: (Error) -> NewError
) -> Result<Value, NewError>

public func flatMapError<NewError>(
  _ transform: (Error) -> Result<Value, NewError>
) -> Result<Value, NewError>

These should probably be proposed separately.

public func fold<Output>(
    onSuccess: (Value) -> Output,
    onFailure: (Error) -> Output
  ) -> Output
public init(_ throwing: () throws -> Value)
public func unwrapped() throws -> Value

I don't think we should include these in the proposal.

My review:

  • What is your evaluation of the proposal?
    +1 for the idea, -1 as proposed
  • Is the problem being addressed significant enough to warrant a change to Swift?
    Yes
  • Does this proposal fit well with the feel and direction of Swift?
    Yes (with some removals)
  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
    No, but I regularly write my own Result type in my own projects.
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
    Followed along with the various pitches, as well as read the proposal.
2 Likes
19

There is no way we would ever make a throwing expression implicitly turn into a Result.

3 Likes
20

<tangent>
... it's a completely natural extension to the syntax. ... throws → Foo should be sugar for ... → Result<Foo, Error>, and try syntax should be sugar for unwrapping the returned result. Similarly, I should be able to try an expression that returns a Result<A, B> as a way to bypass the B to get the A.

This is how the async api should work as well. It should all be sugar syntax around a Future, or something like it.

This way people who want the Result can get the Result, and people who don't can try them away. It makes the language far more composeable and flexible.

</tangent>

11 Likes