GetValueOrDefault?

[dominikjeske]

I'm searching the Result type for something like below but I cant find this functionality. Is there any way to invoke Result<T> type with something like GetValueOrDefault(T default) so when it is success it return it internal value but on error it return given default?

[vkhorikov]

No, there currently isn't. There also should be GetValueOrThrow for situations where you want to throw an exception if the value isn't there. This second one will behave similarly to .Result but is going to be more explicit.

Same 2 methods need to be added for Maybe.

I'll work on this this weekend.

[seangwright]

I think this already exists for Maybe as the overload to Unwrap() that (optionally) takes no parameters.

public static T Unwrap<T>(this Maybe<T> maybe, T defaultValue = default(T))

Would it make sense to call the extension the same thing for Result?

Result<int> resultInt = Result.Failure<int>("Oops");
Result<string> resultString = Result.Failure<string>("Oops");

int valueInt = resultInt.Unwrap();
string valueString = resultString.Unwrap();

Console.WriteLine(valueInt); // "0"
Console.WriteLine(valueString); // "null"

[vkhorikov]

I would rather rename Unwrap as GetValueOrDefault (and add a new method GetValueOrThrow), just so that we have symmetry in naming. What do you think?

[dominikjeske]

For me GetValueOrDefault is better than Unwrap - naming is important ;)

[seangwright]

That brings up a point for discussion - should Maybe and Result share operations and use the same language for discoverability? Or, should they diverge based on their purpose?

Personal opinion - I'd prefer the share naming, and this library is definitely not meant to be analogous to all the operations made available by LINQ, so I don't see a need to align with its patterns (eg XOrDefault()).

[Razenpok]

Being similar to LINQ brings a benefit of less steep learning curve for new developers. I think it might be worth considering, though the library tends to be F#-like, so it might be whatever.

[seangwright]

That's a good point. Is it fair to say, when it comes to naming, the number 1 priority is "Predictable naming within the library"?

If that's the case is number 2 "Predictable naming within the larger C# ecosystem" or "Predictable naming within the functional programming ecosystem (with a nod to F#)"?

[vkhorikov]

I personally wouldn't worry too much about learning curve (since it's a one-off investment). The long-term maintainability is more important. For maintainability, consistency is key. We've already made a lot of naming decisions in favor of F# naming style, so I suggest to continue moving in that direction, unless there are some obviously better naming choices.

Unwrap was in the library from the very beginning and it was the name that first came to mind, I didn't do any research when choosing it. I'm not that familiar with F# and don't know if there's a function with the same name (quick googling says there isn't), so given that there's none (correct me if I'm wrong), we can choose something ourselves.

Here's my train of thought:

1. I don't quite like that the fact that .Value may throw isn't obvious from its naming. I'd like to add another extension method for both Maybe and Result, called GetValueOrThrow that would do the same thing as .Value.

2. Given point 1, it makes sense to also have a method called GetValueOrDefault for the alternative (non-throwing) behavior.

Personal opinion - I'd prefer the share naming, and this library is definitely not meant to be analogous to all the operations made available by LINQ, so I don't see a need to align with its patterns (eg XOrDefault()).

I too prefer to share naming between Maybe and Result. It's not that I prefer to stick to LINQ naming conventions (we don't, e.g. Map instead of Select and Bind instead of SelectMany), it's just IMO GetValueOrDefault and GetValueOrThrow are a good fit in this particular case.

Thoughts?

[seangwright]

Is the idea then to:

1. Add GetValueOrDefault<T>(T default) and GetValueOrThrow() to both Maybe and Result (and GetErrorOrThrow() to Result)
2. Keep Maybe.UnWrap() and Maybe.Value, Result.Error, and Result.Value, but eventually deprecate them and encourage developers to use the GetValueX methods?

I'm ok with GetValueX methods as long as we add them consistently to both Maybe and Result and make it clear which ways are the recommended ones to use the features of the library.

[Razenpok]

Apparently, the F# analog is also called defaultValue, so GetValueOrDefault is fine I guess.

I like the idea of adding GetXOrDefault and GetXOrThrow as members as opposed to them being extensions, and also deprecating Value and Error. I think this is more of a "success pit" way than it currently is

[dominikjeske]

@vkhorikov I agree with GetValueOrX - I like those names because they are simple and self-explaining what they do.

[vkhorikov]

Is the idea then to

Yes, that was my idea. Though, I'm not sure about marking Maybe.Value, Result.Error, and Result.Value as obsolete, at least at first, but we can put a note in XML comments to encourage the use of GetXOrThrow instead.

I'm ok with GetValueX methods as long as we add them consistently to both Maybe and Result and make it clear which ways are the recommended ones to use the features of the library.

Yep, that's also the idea -- make this consistent across both classes and encourage their usage over Unwrap and .Value.

Apparently, the F# analog is also called defaultValue, so GetValueOrDefault is fine I guess

This is the method I couldn't find. We are good F#-styling-wise then.

I like the idea of adding GetXOrDefault and GetXOrThrow as members as opposed to them being extensions

Sounds good, this would make them look as first class citizens indeed.

[guythetechie]

As discussed in #330, can we add the following overloads?

public static T GetValueOrDefault<T>(this Maybe<T> maybe, Func<T> selector)
public static Task<T> GetValueOrDefault<T>(this Maybe<T> maybe, Func<Task<T>> selector)

[vkhorikov]

@guythetechie Yep, that's a good suggestion. Feel free to submit a PR. A couple of things we agreed on in this thread:

• GetValueOrDefault will obsolete Unwrap: Unwrap will need to have an [Obsolete] attribute and call GetValueOrDefault internally
• GetValueOrDefault needs to be an instance method, not an extension, to show that it's a first-class citizen alongside with .Value

[Razenpok]

@vkhorikov Wouldn't it be better to have GetValueOrThrow instead of the .Value? It would be more consistent with GetValueOrDefault and wouldn't give a programmer a false sense of safety which .Value does.

[vkhorikov]

Yes, GetValueOrThrow should be there too. I was referring to GetValueOrDefault because that's what @guythetechie wrote about.

[vkhorikov]

Reopening to not forget to add the same for Result

[Razenpok]

After giving a bit of thought to GetValueOrThrow() and GetValueOrDefault(), I figured that these operators are the railway end - you can use them to exit the Result<T> execution scope. For example:

Result<Config> LoadConfig() { }

Config LoadDefaultConfig() { }

var config = LoadConfig().GetValueOrDefault(LoadDefaultConfig);

But what about plain Result? Should it have something like OnFailureCompensate?

Result ConnectToServer() { }

void StartOfflineMode() { }

ConnectToServer().OnFailureCompensate(StartOfflineMode);

Or maybe it's all wrong with GetValueOrThrow() and GetValueOrDefault()? 😅

[vkhorikov]

You mean movie OnFailureCompensate to Result so that it's an instance method? I don't think it's needed, there's no value in Result, so GetErrorOrThrow() and GetErrorOrDefault() would be enough.

By the way, we need to rename OnFailureCompensate into something from F# terminology (as well as other remaining OnFailure... methods).

[Razenpok]

Yeah, I agree that moving OnFailureCompensate to Result is a bad idea.

I'm talking about the approach (or philosophy) here: if we give a user a way to exit the railway in Result<T>, should we also give one for Result?

[vkhorikov]

Well, there are two ways to do the exit: .Value or .Error. Since, there's no Value in Result, the only option is to exit via GetErrorOrThrow() or GetErrorOrDefault(), which we will add there eventually.

[seangwright]

You can also exit Result via void Match(...);, T Match<T>(...);, or T Finally<T>(...).

[Razenpok]

@vkhorikov Technically, Result.Value is Unit (or void). If one would want to compensate into some default value in Result<T> to bring them from Error to T, then it only makes sense that they would want to compensate with some action in Result, which brings them from Error to Unit

@seangwright Yeah, but none of them are as concise as "Please execute this action if the input Result is Error and return Success"

Anyway, I see that this case doesn't click right away, so I suppose it may be redundant or non-aligned with the main usage patterns. I think it can be dealt with later on if the need ever arises.

[seangwright]

@Razenpok agreed - I just wanted to point out there are a handful of ways to exit the railroad here.

I've been working on a TypeScript implementation of this library, and I've already incorporated the GetValueOrThrow/GetErrorOrThrow changes by making Result<T,E> the standard Result type, with Result<Unit, string> being just another closed generic instead of having a separate/special Result type in this library, which simplifies a lot of these questions.

I think incorporating Unit as part of Result so that it has the same API as the rest of the types seems like a good direction to go.

[vkhorikov]

@Razenpok In your example above:

ConnectToServer().OnFailureCompensate(StartOfflineMode);

What's the signature for StartOfflineMode?

@seangwright

I think incorporating Unit as part of Result so that it has the same API as the rest of the types seems like a good direction to go.

The main advantage of Result over Result<Unit, string> is that it's shorter. It might be possible to keep this advantage in TypeScript without having a separate Result class (as far as I remember, the type system in TS is more advanced and might allow for more syntax sugar), but not in C# unfortunately.

[Razenpok]

@vkhorikov

Result ConnectToServer() { }

void StartOfflineMode() { }

ConnectToServer().OnFailureCompensate(StartOfflineMode);

[vkhorikov]

@Razenpok Yeah, looks good. Not as an instance method, though :) The only issue I have with this code is the naming for OnFailureCompensate. It's a relict from the times when we had OnSuccess and OnFailure extensions instead of Map, Bind, and MapError.

[Razenpok]

What do you think about proper naming then?

[hankovich]

I have a couple of overloads for OnFailureCompensate (which I'll share shortly) and I named them just Compensate. I think it's succinctly but meaningful.

[vkhorikov]

Compensate sounds good to me.