[Bikeshedding]: Supporting throwing transforms on Result


(Jon Shier) #1

During the review of SE-0235, which added Result to the standard library, the core team felt that there was no satisfactory name for a transform that supports throwing closures. Without such a transform, Result is left with this abomination:

let network: Result<Data, Error> = .success(responseData)
let decoded: Result<Response, Error> = network.flatMap { (data) in
    Result {
        try JSONDecoder().decode(Response.self, from: data)
    }
}

This is the simplest version of this transform and already it's highly confusing and easy to produce compiler errors due to type mismatches. More complex transforms are even worse. So I'd like to come up with a suitable name for the operation.

To my mind, flatMap works fine, as we're flattening either a Result into a Result or a throwing closure into a Result. I think the equivalence makes sense and the overload would be pretty easy to add. I also thought about compactMap, but it doesn't really make sense given the current usage, as nothing could be removed.

Any other ideas?


(Cory Benfield) #2

This is not a million miles away from the situation NIO has with its EventLoopFuture type, which in NIO 2 will have the following functions (and the equivalent Error versions for transforming the error case):

public func map<NewValue>(_ callback: @escaping (Value) -> (NewValue)) -> EventLoopFuture<NewValue>
public func flatMap<NewValue>(_ callback: @escaping (Value) -> EventLoopFuture<NewValue>) -> EventLoopFuture<NewValue>
public func flatMapThrowing<NewValue>(_ callback: @escaping (Value) throws -> NewValue) -> EventLoopFuture<NewValue>

Our spelling of what you're proposing is flatMapThrows, so it may be worth adding that to the list of prior art. If the community decides to go a different way with that name, we'll likely adopt the consensus name in the future for our version.


(Michel Fortin) #3

My preferred way to handle cases like this is to use network.get() to propagate the error to the newer result via throwing:

let network: Result<Data, Error> = .success(responseData)
let decoded = Result {
    try JSONDecoder().decode(Response.self, from: network.get())
}

I just have to write the happy path and let the errors propagate themselves. But I understand this style might not be to everyone's liking.


(Benjamin Mayo) #4

In my Swift 5 branches, I am provisionally using attemptMap (behaves the same as map with a throwing closure and returns Result<NewSuccess, Error>) for lack of a better name.


(Svein Halvor Halvorsen) #5

I’m not even sure if I advocate it, but to me tryMap feels better aligned by current terminology, should we go down this path.

Although I think I prefer to just overload flatMap.


(Benjamin Mayo) #6

Yeah, I'd be happy with any throwing overload that lets the client return NewSuccess (or throw) in the transformation closure. flatMap seems appropriate.


(John McCall) #7

This is exactly what I would recommend, and I really don't understand the resistance to it.


(Jon Shier) #8

It’s not possible to use this pattern in chained transforms, as you always have to break the chain to do it. At this point Result has nearly 5 years of usage in the community, so I’m trying to match how it’s usage has evolved in that time to the constraints of the standard library. Using get as suggested has been possible for a long time and yet has not caught on, suggesting that it’s not how most Result users want to use the type.


(John McCall) #9

The only general argument against it that I know of — e.g. that get() erases the stronger type information of a Result — seems to go out the window when you merge in an arbitrary Error from a throwing transform.

If it's easier to apply multiple transforms to a value when it's bound up in a Result than it is to apply them directly to a value, that seems like a serious usability problem in Swift. But it's hard not to suspect that this is not true and people are just avoiding the idiomatic solution because they're transliterating patterns from other languages.


(Gwendal Roué) #10

Hello,

"Idiomatic" doesn't sound quite right, honestly. Both Alamofire's and Antitypical's Result had support for flatMap/tryMap accepting a throwing closure, and I think they are totally entitled to define what "idiomatic" should mean here. It's not surprising that people come back when usual Result patterns are no longer provided for free.

And here it looks like replacement patterns are less good (as explained by @Jon_Shier). Method chaining is one of the many Swift idioms, not some foreign transliterated pattern.


(Xiaodi Wu) #11

@John_McCall is not stating that method chaining isn't idiomatic Swift. You sliced out the first part of the paragraph:

The idiomatic way to apply multiple transforms in Swift while propagating errors is to apply the transforms directly to a value. If that's not easier than applying multiple transforms to a value wrapped in a Result, then we have a big problem in Swift.


(Gwendal Roué) #12

I don't understand. Nobody has any problem with applying multiple throwing transformations. The problem lies in the frontier between Result-based apis, and regular throwing methods (which are the idiomatic way to transform values with eventual error). It is very simple, and has been well explained: there is no point stating it again.

A flatMap with a throwing closure has been requested during the pitch, the review, and regrets have been expressed after SE-0235 had shipped without it. The rationale did not close the door:

Edit: it's just a discussion about a convenience API, nothing more... There is no "big problem in Swift"...


(John McCall) #13

I don't think Alamofire and Antitypical get to define the right idioms for working with Result. Swift is allowed to be opinionated, and the existence of a few longstanding methods (with users, even) doesn't override that. That said, of course I don't unilaterally speak for the entire project; I am providing my opinion, not trying to shut the discussion.

My opinion is that any feature which combines multiple kinds of propagation in an expression is bound to have confusing interactions, so it's better to "canonicalize" to a single kind of propagation, which probably ought to be error-throwing because it composes more naturally. And the counter-argument was that it doesn't compose more naturally because chaining flatMap-like transforms is better than just being able to use the value directly as an expression operand.


(Gwendal Roué) #14

I must here confess that my own usages of flatMap with throwing closures had always looked a little too magical to me, and that I didn't feel 100% comfortable. It's very compressed: there are too many layers to decipher for a newcomer.


(Xiaodi Wu) #15

It is not very simple to me; if it has been well explained, I have not seen that explanation. So indeed: it should be stated again.


That brings me to the quoted part of the decision and @Jon_Shier's interpretation:

That is not how I read the decision note. My reading is that the core team was undecided as to whether adding such a transform would be acceptable under any name, not that they were undecided as to what name. Otherwise, the caveat at the end of their decision ("especially as there are alternative means of expressing this using the catching initializer") would be meaningless:


(Jon Shier) #16

They shouldn't be used to constrain idiom going forward, but they are good examples of what the community's idioms were before Result was added to the standard library. In fact, Alamofire's Result didn't have any functional transforms for years before they were added by a contributed PR, which is rather rare. This illustrates the expectations of users who have used Result in Swift before, as well as the APIs they used.

That it's easier to transform Results by staying within Results shouldn't really be surprising, any more than it would be surprising that transforming a String is easier with API on String instead of passing it somewhere else. The simple fact of the matter is that once in a series of functional transforms, it's easiest to stay there, which shouldn't be a surprise to anyone given the patterns common usage. That it may affect the purity of Swift's error handling idiom is unfortunate but was anticipated by the Error Handling manifesto itself.

I think previous community experience here shows this to be a non-issue. For years users have had little trouble converting between throwing and non-throwing contexts, whether using Result or not.

That's a bit of a strange interpretation. Given the earlier context not shown here, it's clear to me the core team felt that overloading flatMap was unacceptable and searched for another name that could fit. Not finding one, and feeling that the throwing initializer was enough (which I disagree with), they removed it from the proposal.

In any event, the intent of this thread was not to relitigate the existence of a throwing transform at all but to merely explore possible alternate names for such a function. We can argue about its existence if I feel like we've found a good name an create a real pitch / proposal for it.


(Jon Shier) #17

I do wonder if an overload with the same name but completely different closure type, whether throwing or not, would run afoul of the core team's desire not to overload based on whether a closure is throwing.

func flatMap<NewSuccess>(_ transform: (Success) -> Result<NewSuccess, Failure>) -> Result<NewSuccess, Failure>

vs.

func flatMap<NewSuccess>(_ transform: (Success) throws -> NewSuccess) -> Result<NewSuccess, Failure>

Of course, this would also have to deal with the possibility of going from a specific Failure type to Error if the transform fails, but it seems like this isn't a true overload based on throwing vs. not, but instead a rather different type signature altogether.


(John McCall) #18

Xiaodi's interpretation is correct. There were three possible positions: (1) we should add throwing-closure transforms as overloads, (2) we should add throwing-closure transforms with a different name, or (3) we should not add throwing-closure transforms. The core team rejected (1) but didn't reach a consensus between (2) and (3), so the easy resolution was to wait and let the community decide how much it mattered, since APIs can always be added later.

I am in camp (3).


(Cory Benfield) #19

This proposal tends to encounter issues with the type checker. In particular, if you have a closure that both returns a Result and throws, the compiler errors are utterly indecipherable.


(Benjamin Mayo) #20

With a method signature:

public func flatMap<NewSuccess>(_ transform: (Success) throws -> NewSuccess) -> Result<NewSuccess, Error>

I don't think there is any overload ambiguity here?