Add value property to Result

Hi Swift Evolution,

The shortest of pitches...

Introduction

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.

Effect on API resilience

None.

Alternatives considered

None.

47 Likes

I don't have any objections, just curious why?

1 Like

Excellent idea :+1:

This makes Result consistent with Task where we introduced (try) await task.value so it's quite nice to have parity between result and task here :+1:

7 Likes

While cool, what are the (future) benefits over the get method that would justify this addition / change other than relying on new throwing getters?

  • it's not shorter to type (still 5 characters)
  • key-paths don't support a Failure type (at least not yet, if ever)

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().

4 Likes

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'.

10 Likes

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
      }
    }
  }
}
17 Likes

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.

@bzamayo gives the right answer here:

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.

16 Likes

Edit: Smarter minds than mine have prevailed; I'm fine with this.

My original (and incorrect) objections

I'm -0.5 on this, for two primary reasons:

  1. I think it should be called .success (and not .value) in order to match the name of the generic type

  2. 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.

Wouldn't that rather be:

if let foo = try? result.success.foo

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.

Fair enough.

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.

+1 sounds good to me :sweat_smile:

7 Likes

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? :man_shrugging:

4 Likes

+1 this is a nice win! It makes the past version more in-line with what we have learned is more ergonomic.

1 Like

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.

"get"?
https://developer.apple.com/documentation/swiftui/binding/init(get:set:)-7ufcp
"catching body"?
https://developer.apple.com/documentation/swift/result/3139399-init
…"value"?

I say the compiler needs to be improved first, to support Result being elevated to property wrapper status. Then the naming will take care of itself.

Here's what we need:

public var wrappedValue: Success {
  get throws { try get() }
  set { self = .success(newValue) }
}

Here's why we can't have that yet:

  1. // Property wrappers currently cannot define an 'async' or 'throws' accessor
  2. 'set' accessor is not allowed on property with 'get' accessor that is 'async' or 'throws'

The get method can be deprecated after get/set accessors are made accessible as closures.

1 Like

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.

5 Likes
Terms of Service

Privacy Policy

Cookie Policy