[Accepted with Modifications] SE-0235: Add Result to the Standard Library

[Matt_Gallagher]

I love it and agree with all points @John_McCall made (including the complexities behind the decisions).

[mrc (Marco Capano)]

I really like how case let .failure(error) looks, but I'm a bit concerned about get(). It feels a "stranger" in Swift and far away from any of the existing name conventions. Not saying that new conventions should not be created, but this just doesn't feel to be the case (IMHO).

[IanPartridge]

I prefer get() to unwrapped(). To me, wrapping/unwrapping in Swift refers to optionals, which is not what's going on here.

[mrc (Marco Capano)]

Agree on unwrapped() because it is awkwardly reminding the user of optionals. Maybe resolved() or something similar might do better. Missed some of the comments in the review, has `resolved() been discussed?

[IanPartridge]

@John_McCall

Success and Failure are reasonable choices for the type parameter names. Failure is particularly nice because it's a problem to have both an Error parameter and a global Swift.Error type: not because the shadowing is difficult for the compiler, but because the reuse of the name makes code harder to read and — perhaps more importantly — makes it unnecessarily awkward to talk about in person. These names are also rarely used in code, as they only ever need to be written within extensions of Result .

I agree - the Error: Swift.Error generic placeholder name was a problem.

I think once Result is added to the stdlib we should take a look at other places which currently use Result as a generic placeholder name, for example Array.reduce().

Its signature is currently:

func reduce<Result>(_ initialResult: Result, _ nextPartialResult: (Result, Element) throws -> Result) rethrows -> Result

I think there is a case to be made for avoiding the use of Result in this context as it can be confusing, especially to those learning the language.

[benrimmington]

@rxwei

I'm disappointed to see get(), as this is probably the first function name with "get" in the standard library, and it's inconsistent with existing naming conventions.

An alternative is to make Result callable, with an empty [Never] array of arguments.

@dynamicCallable
public enum Result<Success, Failure: Error> {

  case success(Success)
  case failure(Failure)

  public func dynamicallyCall(withArguments _: [Never]) throws -> Success {
    switch self {
    case let .success(success):
      return success
    case let .failure(failure):
      throw failure
    }
  }
}

I've tested this using a Swift 5.0 snapshot.

let result = Result<Int, Never>.success(42)
assert(try! result() == 42)

If subscripts could throw errors, another alternative would be a nullary subscript() throws -> Success.

[nuclearace (Erik Little)]

I don't really like the idea of making result dynamicCallable. I don't think it reads as nice as a method, and it's not as easily discoverable. It also feels like it's conflating the original purpose of dynamicCallable for something that can, and should be expressed as a method.

[jrose (Jordan Rose)]

I'm not sure I like this as a general principle, but if you stick the word "type" after a generic parameter it makes more sense: "Result's Success type", "Result's Failure type". This can also apply to some of the generics we have in the standard library already: "Optional's Wrapped type".

I don't love it though because plenty of types don't need it: "Array's Element", "Dictionary's Key", "UnsafePointer's Pointee".

[nathanhosselton]

I enjoy get() and am v happy to see the case names reverted for the acceptance! I loved the extended justification provided for all of the final decisions as well. Much appreciated.

[DevAndArtist (Adrian Meister)]

Not sure whom you replied to, but personally I would never say result holds a success or result holds a failure, because it's missing something like value at the end, I would think. Although result holds a failure sounds kind of 'okay' to me as a non-native English speaker. I'd rather say something naturally like result holds/contains a value or result holds/contains an error. My grammar skills are probably bad anyway. So I guess this ship has sailed now.

[John_McCall]

Note that get() on Future would presumably be an async operation, so in actual source code this would look like await response.get().

[John_McCall]

Assuming we don’t just do the C# thing and make that await response, of course.

[rxwei (Richard Wei)]

For a long time I believe that "get" in a function name generally lacks a clear meaning, and I have been avoiding "get" in all my Swift code I've ever written.

1. For methods named "get", e.g. x.get(): What is it getting? Without knowing the type of x, I can only interpret this expression as getting x itself, which, if true, defeats the purpose of having such a method.

2. For methods starting with "get", e.g. x.getValue(forKey:): "get" is meaningless in that it does not provide any additional clarity than value(forKey:). I think the Swift API design guidelines solved that problem elegantly by recommending the use of nouns and past participles. Personally, I've been following this convention in all my Swift code.

get() in Result is exactly problem 1. Moreover, if our mental model is to treat Result as a union of the value on success and the error on failure, get() is even confusing when we do see the type. For example:

let x: Result<String, InterpreterError> = ...
try x.get()

It is not clear to me what the second statement is "getting" from x.

Here's my thought process on what to name this method:

• If it's simply retrieving the value on success, why not just call it valueOnSuccess()?

• If the concern is the fact that this method is throwing, then we can see this operation as transforming the value from one representation (the enum Result) to another (the union of a direct return value and a thrown error). This new representation is now part of the code instead of being a nominal data structure, and it can be thought of as a flattened Result. In this case, maybe flattened() is a reasonable name to consider.

• Coming back to call sites, x.flattened() can be very confusing, but x.valueOnSuccess() is crystal clear to me. So I believe names along the lines of valueOnSuccess() are better.

[nuclearace (Erik Little)]

@rxwei

valueOnSuccess()

My one complaint with this naming is that, at least to me, it reads more as good name for an async method. But we don't have those in Swift yet. So a name along the lines of successValue(), which doesn't carry any implication of waiting would be better.

[John_McCall]

Both of those names strongly connote just returning the success value or nil to me.

Anyway, we're not really designing this anymore.

[rxwei (Richard Wei)]

@John_McCall

Anyway, we're not really designing this anymore.

I can accept the reality if this is no longer up for discussion.

That said, I hope that the API design guidelines can be improved to address issues around "get". Many developers who are new to Swift naming conventions use "get" in their function names, mostly in places where the name should be a noun according to the guidelines, but the guidelines today do not provide an explicit recommendation on

1. why and when not to use "get", and
2. when to use get() as a full method name.

[jarod (Jarod Long)]

I'm excited for this to be standardized. I appreciate everyone's hard work in pushing this through!

[Ben_Cohen]

@Aciid

I brought this up before in one of the discussion thread but haven't been able to follow up. The source compatibility section in the proposal is way too vague. I am not sure what happens to existing source code when this type is introduced in the stdlib. For e.g., what happens in SwiftPM itself? If existing source code will break then maybe type should be gated on swift-version 5+.

We have a long-standing practice of not considering additive changes that happen to shadow existing types/functions "source-breaking". As such, we would not normally gate this addition under Swift 5. The main reason this practice is important is we want new features to be available as soon as they can be. We don't want to force someone to update to the newer Swift version (and do the associated migration) before they're able to use new stuff.

Now, we could make an exception in the extreme cases where something is so common and popular a type name that there's a lot of breakage in the short term. But I'd want to play that out. In the case of the package manager, it just needs to update to specify the module name of the non-std lib version to resolve any ambiguity.

[Aciid (Ankit Aggarwal)]

@Ben_Cohen

We have a long-standing practice of not considering additive changes that happen to shadow existing types/functions "source-breaking". As such, we would not normally gate this addition under Swift 5. The main reason this practice is important is we want new features to be available as soon as they can be. We don't want to force someone to update to the newer Swift version (and do the associated migration) before they're able to use new stuff.

Now, we could make an exception in the extreme cases where something is so common and popular a type name that there's a lot of breakage in the short term. But I'd want to play that out. In the case of the package manager, it just needs to update to specify the module name of the non-std lib version to resolve any ambiguity.

That's a little unfortunate (in this case) as it'll cause a lot of source breakage and require people to change their source before they can even use the newer tools. That is much more worse than having a newer feature available in the same swift-version. It is especially bad for the package ecosystem because you usually don't own your dependencies and you've to wait until all your dependencies rename their Result type before you can use the newer compiler.

[nonsensery (Alex Johnson)]

On the topic of source compatibility...

Wasn’t there an issue where it was impossible to disambiguate between a type in the standard library, and a module with the same name?

That is, if you were using a module called Result and then the std lib added Swift.Result, there would no longer be any way to reference the Result type from the Result module. Result would reference Swift.Result, and Result.Result would try to reference Swift.Result.Result.

Was that a real thing? Is it still a real thing in Swift 4? It could be a problem for all of the people using antitypical/Result.