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

ValueOr and AsMaybe for results #594

Open
RorySan opened this issue Jan 10, 2025 · 8 comments
Open

ValueOr and AsMaybe for results #594

RorySan opened this issue Jan 10, 2025 · 8 comments

Comments

@RorySan
Copy link

RorySan commented Jan 10, 2025

Hello, I wrote two simple extension methods that are helping me in my project, but I'm guessing there is probably a better alternative already built in that I am not using:

    public static Maybe<T> AsMaybe<T>(this Result<T> result) =>
        result.IsSuccess ? result.Value : Maybe<T>.None;
    
    public static string ValueOr<T>(this Maybe<T> maybe, string alternativeValue) 
        where T : SimpleValueObject<string> => 
        maybe.HasValue ? maybe.Value : alternativeValue;

I found a way of using the existing Or to achieve this, but it is a bit more verbose. With this ValueOr i get to write things like Email.ValueOr(string.Empty) which is handy when displaying optional info on the UI. Am I missing a better way of doing this?

@bothzoli
Copy link
Contributor

Hey @RorySan,

There's a GetValueOrDefault method defined on Maybe that you could use instead of your ValueOr.

public T GetValueOrDefault(T defaultValue)
{
    if (HasNoValue)
        return defaultValue;

    return _value;
}

For the AsMaybe I did not find anything that would do the same thing, but I may be wrong.
It sounds like a useful enough method.
@vkhorikov or @hankovich do you if there's such a conversion from Result to Maybe and if not, why not?
Would it make sense to include that?

@RorySan
Copy link
Author

RorySan commented Jan 15, 2025

Thanks for your reply @bothzoli , although I think GetValueOrDefault wouldn't work in this case because I am not returning T but string. I'll get back to you with the detailed use case and we can decide if it might make sense to include it.

I've been exploring functional programming for a while now and I use this library extensively. I'm not very experienced with contributions, but I'd be happy to help if that's ok.

@bothzoli
Copy link
Contributor

In the extension method you sent above, the generic type constraint makes T a SimpleValueObject<string>, which in turn makes the T of the public T Value of the value inside the Maybe a string.
This sounds a bit confusing, but the point is, I believe you could as well use a Maybe<string> instead of the Maybe<SimpleValueObject<string>>.
In that case the T returned by GetValueOrDefault will actually be a string, as you need it to be.

This all however makes me wonder what you use the ValueOr method for? This method would "unwrap" the Maybe. Why not just Map over it?

@vkhorikov
Copy link
Owner

  1. For ValueOr, there's an Or extension that accepts Maybe and returns Maybe, and there's also GetValueOrDefault that accepts T and returns T

GetValueOrDefault wouldn't work in this case because I am not returning T but string

You can do a Map before, as in:

Maybe<MyValueObject> myMaybe = ...
myMaybe
    .Map(x => x.Value) // Maybe<string>
    .GetValueOrDefault(string.Empty) // string
  1. I don't think we have an AsMaybe that works on top of Result, we can add one.

@RorySan
Copy link
Author

RorySan commented Jan 15, 2025

I see @vkhorikov . I'll tinker a bit with my use case and let you know. Your solution seems right, although I think it made sense for me to have a custom method for brevity given I use it a lot on my UI. That does not justify adding it to the library tho.

Regarding the AsMaybe for Result, shall I prepare a PR with the functionality and tests and send it your way? Seems small enough for a first timer 😅. Is there anything special I should take into account? Thanks!

@vkhorikov
Copy link
Owner

Yep, feel free to submit a PR please

@RorySan
Copy link
Author

RorySan commented Jan 17, 2025

I've sent the PR. I'm not experienced at all in these matters, this is my first PR to an open source project. If I'm doing something wrong or not following best practices, please do not hesitate to point it out. Thanks.

@bothzoli
Copy link
Contributor

Adding PR reference #597.
I was only able to skim through it and so far it looks good.
I wonder if it'd make sense to add the extension method to the other result class variants (UnitResult, Result<T,E>)?
What do you think @vkhorikov?

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

No branches or pull requests

3 participants