maxveldink / sorbet-result Goto Github PK

Thanks to @iMacTia 's awesome work on the and_then chain, we have a great composable way to work with Results. If a Failure is encountered, we end the chain (rather, perform no-ops the rest of the way) and return that Failure. It would be great if we could pass a block to be executed if a Failure is encountered (useful for error logging or other reporting). It's important that we pass the Failure back instead of the return type of the block, as we want to guarantee the same Result type as and_then or the original Result.

res = do_something
  .and_then { |success| Failure.new }
  .on_error { |failure| puts "An error" }

# prints "An error"
res # => Failure

Implementing the == method on results, successes, and failures would be nice, especially for test frameworks. I took an initial stab at this and was met with some generic variance errors. I would be happy to have a contribution for this if someone wants to give it a shot.

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.

Ruby 3.0 is now more than three years old and I'd like to target 3.1 as the minimum supported version.

One pattern that's emerged as I've used Results in other projects is having to write Minitest assertions like this:

result = do_something

assert_predicate(result, :success?)
assert_equal("payload", result.payload)

I'd like to have an easier facility for this in minitest like:

assert_success(result)
assert_payload("payload", result)

We have a fairly extensive README that details how to use many of the features of Results, but we could benefit for some deeper comments and examples above each method.

There's a common concept in functional programming called flat-mapping or chaining that can help making code easier to follow in certain situations.

Take this example:

result1 = FirstService.new(...).call
final_result = if result1.success?
                 result2 = SecondService.new(result1.payload[...], ...).call
                 if result2.success?
                   ThirdService.new(result2.payload).call
                 else
                   result2
                 end
               else
                 result1
               end

# can be a bit easier if you can return
def complex_operation(...)
  result1 = FirstService.new(...).call
  return result1 unless result1.success?
  
  result 2 = SecondService.new(result1.payload[...], ...).call
  return result2 unless result2.success?
  
  ThirdService.new(result2.payload).call
end

With chaining, the same can be expressed in the following way:

result = FirstService.new(...).call
  .flat_map { |res| SecondService.new(res[...], ...).call }
  .flat_map { |res| ThirdService.new(res).call }

# or nicer, with support from services for `.call` and using the `_1` variable
result = FirstService.(...)
  .flat_map { SecondService.(_1[...], ...) }
  .flat_map { ThirdService.new(_1) }

Here is an example of another gem supporting this syntax: https://github.com/tomdalling/resonad#flat-mapping-aka-and_then

I'd like to know if you'd be open to supporting this behaviour before implementing this 😄

Is there any reason why Typed::Failure and Typed::Success use T.nilable(Error) and T.nilable(Payload) internally?
Since those types are type_member, the user should be in control on making them nilable.
For example, if I define a method like this:

sig { returns(Typed::Failure[String]) }
def method_returning_failure
  # ...
end

I then need to deal with nilability on the caller side:

result = method_returning_failure
if result.error? 
  method_expecting_string(T.must(result.error))
end

The same issue exists for Payload, but in that case at least there's a payload! helper method which makes coding easier, but voids some type-checking assurances in the process and could lead to unexpected errors at runtime.
Removing the use of T.nilable, would allow the class to assume a value is always provided.
If for whatever reason the value needs indeed to be nilable, then we can simply specify that via the generics interface:

sig { returns(Typed::Failure[T.nilable(String)]) }
def method_returning_nilable_failure
  # ...
end