This proposal introduces a throwing value property to access the Success value of a Result.
Motivation
SE-0310 added throwing properties, and so SE-0304
has a throwing value instead of get(). This adds a matching equivalent to Result
to allow for consistent use.
Proposed solution
Add the following to Swift.Result:
extension Result {
@_alwaysEmitIntoClient
public var value: Success {
get throws {
switch self {
case let .success(success):
return success
case let .failure(failure):
throw failure
}
}
}
}
Source compatibility
The removal of the get() equivalent is not proposed, so this change is
source-compatible. The deprecation/removal of get() could be considered
in future, possibly gated on a major language version.
Effect on ABI stability
None. This is ABI-additive. Since the implemention is trivial, it can
be inlined into the client so is not gated on deployment target.
I presume a dislike for the name of get(). Not specific enough, in opposition to the naming convention of functions that return a value (foo() vs. getFoo()), etc. The review of the Result proposal contains a lot of arguments against get().
While true, I just want to understand the justification that the core team has for this change. I don't want to open any pandora box, but I kindly remind the thread of the debacle behind toggle() and the rejection of isFalse on Bool. If anything then Task should have followed the precedent of Result in that measure and not define "the new way"(but that shouldn't be the discussion here ;) ). Again, this isn't meant to blame on anyone, I'm only looking for reasonable justification arguments.
Besides the potential disliking of the get() name and signature, the proposal as for right now failed to explain on why we want this change, because from my consumer's perspective it does not enable anything new nor it improves anything (again other than naming maybe).
Different people have different levels of tolerance for 'churn', but I am all for this renaming. I've always hated get(). If throwing accessors existed when Result was formalised, we would have done it then.
I think it's fine for synonymous things to co-exist with a view for eventual deprecation down the road. It'd be nice if the code documentation for get() could be updated to say something like 'Prefer value spelling'.
Like the Task API, will there also be a non-throwing property?
(I hope this is possible with @_alwaysEmitIntoClient.)
extension Result where Failure == Never {
@_alwaysEmitIntoClient
public var value: Success {
get {
switch self {
case let .success(success):
return success
}
}
}
}
I don't think "debacle" is an appropriate characterization of the toggle review. The core team was clear on the merit of that proposal when accepting it. In fact that was where we codified the criteria for this kind of addition.
The value of a result or a task is very much a property of the type, not a function that requires execution, insomuch as anything deserves to be a property (it doesn't even require awaiting unlike on Task – it's right there for the taking), and had throwing properties existed this is what it would have been called originally.
This is what distinguishes it from isFalse, which is not the "if we had our time again" alternative to !.
This is a good idea – essentially the softest of deprecations. I'll add to the proposal.
I think it should be called .success (and not .value) in order to match the name of the generic type
Using a throwing property makes it more difficult to deal with accessing members on the returned value when chaining; I've found a Success?-returning property to be far more convenient in day-to-day uses.
Or in other words, this is nice: if let foo = result.success?.foo.
This is less nice: if let foo = (try? result.success)?.foo
In the case where I want to explicitly transform a Result into a throwing operation, I don't mind sticking with the try … .get() syntax.
This seems to me like a reason to call it value, in order to provide an optional for the success case (in fact this is a common extension to Result, including in the unit test for this very feature).
This is separate from whether it is worth providing a migration from a function to a property.
Haha true… but then isn't that also an implication that var success: Success? should be public? If it's so common (which it sure seems to be), then shouldn't we be considering exposing that instead?
Ha, you're right. I'm surprised; from what I remember, that didn't use to work.
Since it appears to in my tests, I object less to this. :) (I still think the name is wrong though, and that if var success: Success? is so common then we should be considering that instead)
Yes, it probably is. The answer to why it isn't lies in the acceptance for SE-0235, about which not much has changed since:
Case accessors are a more complex question. The current proposal doesn't include case accessors, but the original proposal did, and we're anticipating a future proposal that will add automatic synthesis for case accessors. That synthesis will undoubtedly use names based on the actual case names, so at a first glance, it seems imprudent to use case names that we're not sure we'd want for the accessors. However, feedback on this review has led the Core Team to more seriously consider this issue, and it now seems quite complicated. A name that's a good name for a particular state of an enum (like failure ) might not be a good name for a value carried in that case (like error ). It may be true that case accessor names will always end up feeling a little contrived, and so it's just better to use the actual case names because they're easy to interpret. More importantly, we don't want to burden this proposal because we haven't resolved those questions yet to our satisfaction.
Given @DevAndArtist's correction to my syntax and my realization that this is a different way of accessing the entire result (not just one case, since this returns either the success or the failure value), I withdraw my objections.
Thanks for humoring me as I stumbled through this.
I’m in favor of this. Not much else to say on the contents of the proposal.
As to the process, though, given that it was the core team that inserted get() in place of unwrapped() on its own accord into the design of Result, writing that it expected a future type to use the same API, and then the core team that inserted value on its own accord in place of get() into the design of that very API they envisioned before, what is the role of the community in addressing this issue?
This proposal might seem innocuous, but it's actually demonstrative of a lack of standardization of dependency-injected get accessors. Something needs to be done here, but proceed with caution, because you're setting a precedent for a pattern that's going to grow in popularity now that properties are growing up.
I like this addition, as long as it doesn't interfere with the possible future addition of typed throws. Losing the Failure type here permanently would be unfortunate.
As suggested previously, it would be a good idea to roll this change together with previously proposed changes to support an async closure initializer and other enhancements to better support the concurrency features.
