seangwright / typescript-functional-extensions

A TypeScript implementation of the C# library CSharpFunctionalExtensions, including synchronous and asynchronous Maybe and Result monads.
MIT License
33 stars 4 forks source link

Introduce safe way to access Result.state-Members directly #11

Closed GregOnNet closed 2 years ago

GregOnNet commented 2 years ago

Is your feature request related to a problem? Please describe. Using getValueOrThrow or getErrorOrThrow is a bit hard to read, even if you have checked before whether the result is in success or error-State.

It would be nice to access value and error directly, when it is clear in which state the Result is.

Describe the solution you'd like After executing a Type Guard, I want to access value or error directly. This should be automatically inferred by the TypeScript Compiler.

Describe alternatives you've considered Currently, I use the existing API for a few weeks by using *OrThrow()-Methods. This approach is not the cleanest. My Co-Workers being new to this library get confused too. They find accessing a value- or error-Object is too complicated.

if (result.isFailure) {
  throw new RpcException(result.getErrorOrThrow());
}

return result.getValueOrThrow();

Additional context

seangwright commented 2 years ago

I like the idea of using type guards to let devs access the properties which would throw if not correctly accessed.

I see some monad libraries not allowing direct access at all and instead consumers have to use .Match(...). This is definitely less ergonomic for simple use cases.

GregOnNet commented 2 years ago

Maybe there is already a way to accomplish my use case without exposing value and error.

Use Case

Is there a way of doing this with match? Honestly, I prefer to stay as long as possible with the Monad.

seangwright commented 2 years ago

@GregOnNet Here's an example of how to use Result.match to achieve what you are trying https://stackblitz.com/edit/typescript-lhgdur?file=index.ts

You can throw in the failure callback for match and the typings will still reflect what is returned in the success callback.

const number = r.match({
  success: (n) => n,
  failure: (e) => {
    throw Error(e);
  },
});

If I'm working in Angular or Vue, I'll use Result.tap() and Result.tapFailure() to update state without having to unwrap the value/error. The closure over this or the state object works just fine, even with ResultAsync:

const apiResult: ResultAsync<number, string> = ...

await apiResult
  .tap(num => {
    this.responseNumber = num;
  })
  .tapFailure(err => {
    this.errorMessage = err;
  })
  .toPromise();
GregOnNet commented 2 years ago

Thanks for the nice write-up @seangwright.

.match

To me, throwing the error inside the Monad feels like escaping the pattern. If I just want to access the value, I also need to specify a success-Callback.

const number = r.match({
  success: n => n,
  failure: e => {
    throw Error(e);
  }
});

.tap/.tapFailure

Interesting approach. The only downside I see here, that I would need to declare a variable and assign it later on. This could again lead into handling undefined values, if I am not mistaken.

let responseNumber: number | undefined;
let errorMessage: string | undefined;

await apiResult
  .tap(num => {
    responseNumber = num;
  })
  .tapFailure(err => {
    errorMessage = err;
  })
  .toPromise();

How it is solved in CSharpFunctionalExtensions?

CSharpFunctionalExtensions allows accessing .Value- and .Error-Property. https://github.com/vkhorikov/CSharpFunctionalExtensions#get-rid-of-primitive-obsession.

Personal conclusion

Comparing all the mentioned approaches, I still believe Result<T> will benefit from introducing Type-Guards. 👍

GregOnNet commented 2 years ago

Hi @seangwright, what do you think to my thoughts?

Cheers 🎉🎉 Gregor

seangwright commented 2 years ago

@GregOnNet I think it's a good idea to make the library more convenient in these situations.

I agree that using throw Error in .match isn't a great approach, I was just showing that it is an approach.

I think throwing errors is a bad approach, in general 😀

Instead I think staying in the monad as long as possible, and then mapping into a single type (even if that type is T | undefined) is the best approach and the one I've used with success in my applications.

I think adding some guidance in the README.md (or another doc file) would help explain when to use the type guards and when other approaches might fit better with the opinions of the library. Updating the docs ties in with #7, which can be done with a later (non-functional) update.