Adding Result II: Unconstrained Boogaloo

I wrote an implementation based on the feedback from this thread which I would like you to consider:

All of the names are fully bikesheddable.

I decided to error on the side of providing too much, since it is much easier to remove things from a proposal than to add them. The part which is most important to me is the interop with native Swift error handling. I use that a lot in my day-to-day use of my own Result type. I probably use what I am calling then and recover most often, and of course value() (which is unwrapped() in the current proposal)

I also tend to use the convenience handlers (e.g. onSuccess{...}) a lot, though I can add them back in pretty easily.

Thanks for putting this together! I’ll start by replying at a high level and then get into some specifics.

At a proposal level, I don’t think this proposal would be well served by trying integrate every bit of feedback given in this thread and elsewhere. To me, that just creates more friction in the community by baking in features people might disagree with, as well as making the implementation that much more complex. My purpose with this proposal is to create the most generally usable Result I can that fits in the Swift standard library. The type, plus a base level of convenience API that is similar to other types in the stdlib is what I’m aiming for here. To me, that means super common Result functionality, like map or initializing from a throwing closure. Not the full breadth of functional API possible for a Result type, nor every possible convenience feature, as most of those can be added later, if popular enough. Ultimately this proposal aims to bring the benefits of Result to everyone while at the same time allowing everyone who’s written their own version to standardize on Swift’s instead.

Moving on to more specific issues; I couldn't disagree more with your typealias. Not only is it unnecessary (you haven't justified it beyond Result<Value> being the spelling you want) but incredibly backwards. Even if I was a fan of such built in alias (I'm not), it should flow from most general to less, not the other way around. Result<Value, Error> to ErrorResult<Value>. But I think offering an alias along with the introduction of the type itself would just cause confusion.

I see little value in init(_ value:) or init(error:), as they're barely any more convenient than just spelling out the case names. init(_ value:nilError:) may be useful, but I'm not sure about making it part of the initial implementation. I think an initializer like init(_ value: _ error:) would be more useful to map from Apple's API's which return optional success and failure values. I'll think about that one further.

optionalValue and optionalError go against the naming guidelines, plain and simple.

I'll be revisiting the various maps with throwing closures, but something like bimap is unprecedented in the standard library, AFAIK, and I don't really want to discuss its value at the moment.

hasValue and hasError seem very awkwardly named to me, and are largely served by having isSuccess. The ability to check for a certain type of error is interesting, but like most of the functionality you've outlined, I think is best left for later, depending on use.

I like onSuccess and onError, and I added them to the Alamofire Result implementation myself (along with ifSuccess and ifFailure), but I think such API should be common across all relevant types in Swift, particularly Optional. I'd like to see Optional add withValue or something, in addition to map, and if that happens, I'd like Result's to line up so users aren't surprised when they move between types.

As for the rest of the functional API, I think it's best to wait for Result to be accepted and then used, and then add popular functionality. Not everyone who uses Result will need to use it with those APIs, so I think it's best to see how it's being used before bundling in more functionality.

So thanks for the work, but I'm pretty happy with the level of functionality the proposal's at right now. If this gets into review, I'll be happy to add or remove whatever functionality the core team feels is necessary for Result to fit into the standard library, but for now I'm going to keep things simple.

6 Likes

I've added back map and mapError that take a throwing closure, constrained where Error == Swift.Error. I agree that a throwing flatMap doesn't really make sense, as any errors should be captured into the transform's returned Result.

One last bit of bike shedding I'd like to do is for an initializer to be used to transform between the doubly optional input of closures, like (value: Int?, error: Error?), mainly to be used to allow for easy adaptation from various Apple APIs that follow that pattern. This could even be for Error? handlers too, since we can have Result<Void, Error>. Examples of APIs to be adapted would be appreciated too.

My initial though is just:

init(value: Value?, error: Error?) {
    print("\(#function) must be called with at least one non-nil parameter.")
    switch (value, error) {
    case let (_, error?): self = .failure(error)
    case let (value?, _): self = .success(value)
    default:
        fatalError("\(#function) must be called with at least one non-nil parameter.")
    }
}

However, I'm not entirely sure about the error over value precedence.

Also, how should I represent the fatal error for a stdlib type?

It should come as no surprise that I strongly object to mapError with two error paths. There is no good reason to allow this. Your proposal will be stronger if you remove it.

Also, as per previous comments I have made, the appropriate name for your map with a throwing closure is actually flatMap. This is the name that matches the semantics it provides. If you want to include that method please use the correct name for it. This method is just a syntactic reshuffling of the traditional flatMap, using throws rather than a result type of Result for the error path.

1 Like

I still don't understand your concern. It doesn't seem confusing to me that if you use a throwing error transform, you're going to get either your intended transform result, or the error that was thrown by it. This isn't any different than any other throwing function that can throw multiple types of Error.

This is only true if you consider a throwing function and a function that returns a Result to be the same. Is that where you're coming from here? I suppose this proposal does sort of establish that equivalence.

I think I understand your points now. If one considers functions returning a Result and throwing an Error to be equivalent, then your point about mapError makes more sense, as well as the expected flattening behavior. My main concern was user confusion when functionality they were expecting wasn't there. I'll take another look.

I've updated to keep and rename map with a throwing closure to flatMap and removed the Error transforming equivalent.

I'd still like to discuss the double optional (or equivalent) initializer.

The concern is that there are two ways to produce the new error. The transform can return it directly or it can throw. Both paths are identical in type and in behavior with regard to the semantics of mapError, therefore they are redundant. This forces a choice on users regarding how to return the new error (directly or by throwing). This choice serves no useful purpose and will lead to unnecessary confusion.

They are just two different ways of reporting an error from a function. In this case, the the error reported by a throwing closure is handled in exactly the same way as the error returned via a Result in the traditional flatMap operation.

Awesome! Thanks for thinking again about this.

I don’t know what you’re referring to. Can you elaborate?

Sorry, wasn't necessarily talking to you, but I was referring to the initializer I mentioned recently, something to help transform previously double Optional APIs into Result closures. Something like this:

init(value: Value?, error: Error?) {
    print("\(#function) must be called with at least one non-nil parameter.")
    switch (value, error) {
    case let (_, error?): self = .failure(error)
    case let (value?, _): self = .success(value)
    default:
        fatalError("\(#function) must be called with at least one non-nil parameter.")
    }
}

I see, I wondered if you meant something like that. I would leave that off for now. It’s easy for people to define it themselves if they want it. It’s also reasonable to want avoid the fatalError in the implementation by making this failable. In that approach callers have the choice of force unwrapping the result wirh the potential trap visible at the callsite or handling the nil case.

I imagine there would be many advocates in both camps with no clear consensus on what is better. It seems better to keep the proposal focused on the core type and let people add conveniences like this themselves if desired.

1 Like

Very true. I'm just thinking about next steps. If this proposal is accepted I'd like to come up with some sort of Obj-C markup to allow capturing separate values as a Result. But that can certainly wait.

Long time reader here, I have been watching these thread(s) and the related PRs for a while, as they intersect closely with some of the things I have been looking at lately. Really happy to see the progress here!!

I'd like to echo support for a few of the spellings above (to be followed by subsequent justification):

enum Result<Wrapped, Failure> {
   case value(Wrapped)
   case error(Failure)
}
  1. Wrapped is prior art from Optional and emphasizes the similarity of intent: "This is where you look for the thing the Result is Wrapping.
  2. Describing things that can never fail is beautifully represented by: Result<Foo, Never> -- and you even get to elide the case-statements in those cases!
  3. having the generic be Failure for the 'error' oriented generic parameter, helps put some distance from Swift.Error - whereas the case being error` still communicates intent.
  4. success is sometimes the perfect name for the non-error case, however it has extra connotation of completeness, which isn't always the case.

Imagine if this being used in some callback that could produce multiple intermediate results (in this case I am using nil as the completion sentinel, similar to Sequence, and Bool as a poor-mans spelling for stop-flag cancellation API):

func stream(this url: URL, _ aCallback: (Result<Data, Error>?) -> (Bool))

If you squint, sure you are 'successfully receiving parts of the payload'. value is not much better but its pretty inert with little connotation other than its something. I suppose that might be an indication we should considering borrowing some(Wrapped) from Optional.

Basically I worry about the connotation of success in situations where Result may be used to convey something where 'success' isn't quite the right cognitive fit.

2 Likes

I don't believe that spelling has any significant advantages over the one proposed, but I'll address your points.

Result does not wrap a value. It's a representation of two exclusive states, a success and a failure, both of which can wrap a value. This is especially true in this unconstrained version, where the failure value may not be a Swift.Error. So giving one "side" of the representation a claim on the "wrapped" value is wrong and may be confusing.

Interesting, I'd never seen that before. Though the compiler still lets you switch on the failure case (bug?), it doesn't complain if you don't. Though you get this with both spellings.

Again, I have to disagree. While there some distance, I don't believe it's much, and generally I don't think Failure is a good generic type name. Matching the generic placeholders and the property accessors makes sense to me, and distinguishes the meaning of the cases from the types of their associated values. Besides which, you've contradicted your point about distance from Swift.Error by having your failure case be named error.

Simply having a single Result value implies completeness, so I don't think the case name will matter much in that respect. Not that I think the implication is very prominent. Additionally, value is far too non-specific a case name, and contrasts poorly with error, mainly since they both capture values. I'm not worried about the "cognitive fit" of a success case for streams of Result values. I'm not sure another name would be any less confusing in that regard. Whatever the spelling chosen, both the case names and the generic types should be opposites (for lack of a better word) that explain their use and meaning.

This spelling (cases and generic placeholders) is based on the two most popular Swift Result implementations: Alamofire's Result<Value> and antitypical/Result's Result<Value, Error: Swift.Error>. As such I believe it has the best chance of being adopted by the community and allows largely seamless migration from those implementations to a standard library version.

1 Like

I like the implementation, but imho the proposal could be improved with some examples why Result should be part of the stdlib.
Neither having a separate library for Result, nor repeatedly declaring it when needed looks like a perfect solution, but is this enough to put it into the core of the language?
The motivation would be stronger if the whole community would prefer a single variant of Result, but afaics, there is no consensus about the details (and everybody can easily define the type exactly how he likes it to be).

So is anybody having real trouble because there is no "standard", or are there some common situations where interoperability would be beneficial?

2 Likes

I can see that, but errors aren't the nominal case; the 'value' side (regardless of constraint) remains the 'value' side. That said, I can totally get behind calling it value. In my mind thats what differentiates Result from Either (and its a significant/convenient differentiation) – having the error not just be convention is a useful abstraction to lean on.

I was also surprised to see the compiler not complain about this. I will file a radar/jira about it, since it also doesn't complain once you delete the Never branches.

In this case though, error is just a case name in the Result enumeration. So it has more freedom to be whatever it wants since it is already namespaced. The generic parameter name is also orthogonal from the definition of protocols, but it is a little closer since it is a stand in for a type. Naming the generic after an extant type is the confusion I was considering to avoid. One would never do this because it is just confusing:

func something<StringProtocol>(_ s: StringProtocol)

That said, I appreciate the matching of the case names to the generic, and .error is a very good case name. So I could see that being the right thing to do.

I think the difference in my mind, is that Result can imply completeness, but that it is not intrinsic to all use cases.

Conceding on the Optional prior art then, and using the same argument for Value as you describe for Error, I think this spelling is better, and avoids the 'completeness' issue with .success, and matches many folks' Result implementations.

enum Result<Value, Error> {
   case value(Value)
   case error(Error)
}

What do you think?

I'm really not sure what you're looking for here. The proposal (and the error manifesto before it) lays out a variety of use cases, as well as the philosophical differences between Swift current error handling and Result. So I believe the general usefulness of the type has been well established, as it's popularity within the Swift community attests.

Additionally, I would say that there is, in fact, consensus on the spelling of the type in almost all regards, considering the most popular implementations are almost identical. That popularity is another reason why it belongs in the standard library, in addition to its utility in general. Previous discussions of the type has centered almost solely around typed vs. untyped errors, with the spelling barely coming up. Of the nearly one hundred posts in this thread, and the over one hundred in the last, the spelling of the case names was mentioned a handful of times. To me, that's consensus.

Finally, as standalone types like this become more popular, collisions between user implementations and various library implementations become more common. For Result, we see occasional requests for Alamofire to use antitypical's Result implementation (we don't, since we have a strict no dependencies policy for the library), since they collide, and that library is used as a dependency for a variety of other libraries. All of this would be made much simpler and generally better by including the type in the standard library.

3 Likes

I'm sorry, I just don't see any compelling reason to change the spelling of a type already in use by the community. Not only do I think the proposed and current spelling are well reasoned and fit with the Swift naming guidelines, but given its popularity in the community, if this was an actual point of confusion, I think we would've heard about it by now.

1 Like

I'm not sure what you are looking for with this answer either ;-). I guess there is just to much unconstructive discussion in the Evolution process, which promotes a defensive attitude...
Relax - I never questioned that Result is useful; but in this thread, the utility is only a small aspect:
Things can be very useful without being part of the stdlib, and I couldn't find any arguments in the proposal that require Swift.Result instead of

public enum Result<Value, Error> {
    case success(Value), failure(Error)
}

declared somewhere in the library that's producing the results.

This last paragraph is all I wanted to see.
I can't say if this is really problem (I neither use Alamofire nor any Result-frameworks), but others might chime in here.
Looking through the preceding posts, you can easily get the impression that inclusion of Result is already decided, but imho there's too much bikeshedding and too little arguments.

3 Likes

I think that's the first time I've seen someone ask for more argument as part of the proposal process. :slight_smile:

Much of the basis for this proposal was discussed in the comment chain linked in the first post. Most notably, the unconstrained spelling was proposed in the March '18 comment chain as part of that discussion. So this proposal integrates all of the previous feedback and uses the spelling most accepted at that time. This thread was mainly to bikeshed the additional base functionality needed for a good first version of the type that could be included in the stdlib.

However, I think there are really two big reasons to include it in the standard library.

  1. It's the natural counterpart to Swift's current error handling, as outlined in the error manifesto. Offering it alongside try/throw gives Swift a more complete error handling picture, and prepares the language for the future.
  2. It's a popular type and useful type, not just among contemporary languages, but especially within the Swift community itself. So much so that there are a variety of popular implementations are available that may conflict, in addition to whatever implementations the user may have created directly.

As an aside, while trying to build a toolchain from my implementation, the build failed due to SPM having defined its own Result<Value, ErrorType: Swift.Error> type. One of the interesting things from that version is full Codable support, which I may look into.

3 Likes