Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Ensure null with Maybe #571

Closed
DerStimmler opened this issue Sep 30, 2024 · 7 comments · Fixed by #588
Closed

Proposal: Ensure null with Maybe #571

DerStimmler opened this issue Sep 30, 2024 · 7 comments · Fixed by #588

Comments

@DerStimmler
Copy link
Contributor

Currently, the ToResult method on Maybe returns a success result if the Maybe has a value, and a failure result if there is no value.

But what if it should be the other way around?


Lets say I want to create a report, but only if no report already exists.

Right now I would probably do something like this:

await Result
      .Success()
      .Map(() => service.FindAsync(id)) //returns Report?
      .Ensure(report => report is null, "There is already a report available.")
      .Tap(_ => service.CreateReport())
      ...

But this comes with some overhead because I have to start the chain with an empty success result to then map the nullable value and ensure its null using the Ensure method.

I think it would be more appropriate and compact to use Maybe and its ToResult method since I'm dealing with a nullable.

But to keep the chain running if the Maybe has no value, I basically need an inverted ToResult method that returns a success result if there is no value. E.g:

await Maybe
      .From(service.FindAsync(id))
      .ToInvertedResult("There is already a report available.") //returns success result if Maybe has no value, otherwise returns failure result with error message
      .Tap(() => service.CreateReport())
      ...

What do you think about this addition?

ToInvertedResult is just an example name because I can't think of anything better. 😄

@vkhorikov
Copy link
Owner

Yeah, sounds useful enough. Feel free to submit a PR. Can't think of a better name than ToInvertedResult either :)

@bothzoli
Copy link
Contributor

Hey @DerStimmler, @vkhorikov,
I'm happy to give this one a shot in the coming days.

@bothzoli
Copy link
Contributor

bothzoli commented Dec 12, 2024

If I'm correct with my thinking, this will return a vanilla Result for Maybe<T> and a UnitResult<E> when using a custom failure.
Because I cannot populate a T from a failed Maybe<T> for it to be Result<T> (or populate a T for a Result<T, E> from a Maybe<T,E>).

But I believe it would probably make sense to add overrides which could do just that, like:
Result<T> ToInvertedResult<T>(in this Maybe<T> maybe, string errorMessage, Func<string, T> compensate)
or
Result<T, E> ToInvertedResult<T, E>(in this Maybe<T> maybe, string errorMessage, Func<E, T> compensate)

See #588, any thoughts?

@DerStimmler
Copy link
Contributor Author

Hi @bothzoli,
thanks for your help.

I think you are right with Result and UnitResult<E> as return types.

I don't think we need the compensate func since there is already a Compensate operation.
Then it would look like this:

Maybe.From("Foo")
  .ToInvertedResult("Bar")
  .Compensate(error => ...)

Since we can create Task<Maybe<T>> from async values the ToInvertedResult methods need async variants.

await Maybe.From(Task.FromResult("Foo"))
  .ToInvertedResult("Bar");

Here are some async examples for the normal ToResult method for reference.

@bothzoli
Copy link
Contributor

Hey @DerStimmler,
Thanks for the quick feedback. Yeap I'll make sure that the async use-case is also covered, I just wanted to make sure I understood the issue correctly (hence the PR still in draft).
I'll try to tie up all the loose ends in the coming days and issue the PR.
Cheers

@bothzoli
Copy link
Contributor

@DerStimmler I've finalised the PR, feel free to review.

@DerStimmler
Copy link
Contributor Author

Great, I'll have a look at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants