search

maxveldink / sorbet-result

Adds T::Result as a basic, strongly-typed monad
MIT License
23 stars 2 forks source link

Implement default value for `#payload` #16

Closed maxveldink closed 1 year ago

maxveldink commented 1 year ago

It would be nice to provide a default value to payload that is used if we're on a Failure type. I'm open to making this an optional argument to the current payload method or implementing it as a new method, perhaps named #payload_or. We should guarantee the type of the default value matches the Payload type.

iMacTia commented 1 year ago

Could you provide an example of how this would help in practice? I'm trying to understand the proposal, but my current understanding makes me think this is not possible. Failure::Payload is fixed to T.noreturn, so it can't have a default value or even a value at all right now.

maxveldink commented 1 year ago

I think we'd be able to do it with a separate method. It's similar unwrap_or_else or unwrap_or_default from Rust. In practice, we have several uses where we want something with the Payload type but not do a ternary check to provide a default if it's failure.

success_result = something_that_succeeds # Success[Integer, Integer]
failure_result = something_that_fails # Failure[Integer, Integer]

success_result.payload_or(2) # => 4
failure_result.payload_or(2) # => 2
maxveldink commented 1 year ago

Ahh, playing with this more I'm seeing the issue with the generic types more. Okay, cool I don't think we can do this easily right now and that's fine. The ternary evaluation is fine:

result.success? ? result.payload : 2

iMacTia commented 1 year ago

Ah that would be the equivalent of Scala's get_or_else right?

In that case, I think we could do it this way to avoid issues with the now covariant type_members

maxveldink commented 1 year ago

I arrived at the same idea locally by introducing a new generic type parameter. The only limitation is that the any return type might be slightly frustrating. It would be fine if someone were using the same type, but it could force you to cast. Originally, I had wanted to reuse the Payload type to prevent someone from providing an argument that didn't match the Payload return. Now, I think that is too limiting, and technically we should allow you to fall back to whichever type you wish. Sorbet will still statically check that correctly.

The only other thought is get_or_else would riot folks in the Ruby world lol. Thoughts on naming? Is payload_or_else fine? payload_or is still my preference, but it probably could be confusing.

iMacTia commented 1 year ago

technically we should allow you to fall back to whichever type you wish. Sorbet will still statically check that correctly

Yeah I also immediately went for Payload instinctively and found the issue with the covariant type, but then I had your same thought: why limiting the return type? Let's let the use return whatever they want. If it matches the Payload type then great, you won't need to case or cast. If you use something else, then Sorbet will automatically switch to T.any which means you can still use any method available on both classes (e.g. to_s) without resorting case or cast 👍

The only other thought is get_or_else would riot folks in the Ruby world lol. Thoughts on naming? Is payload_or_else fine? payload_or is still my preference, but it probably could be confusing.

I don't mind about the name at all. If we want to stick to the convention we launched with and_then then we really can call this anything 😄. payload_or seems descriptive enough an short and convenient at the same time 🙌