Naming of ResultExtension BindIf() misleading

[mnissl]

If I'm not mistaken, the idea of Bind is to perform a transformation: the extension method receives a Result<T> and returns a Result<K>.

When I then read BindIf, I have the assumption that a transformation is performed based on a condition. But this cannot work out as the method needed to have two return value types: one if the transformation is performed and one if it is not. As we all know, this is not possible.

So, BindIf is the wrong name: it can never perform a transformation, only a mutation.

BindIf is more similar to TapIf, except that it returns a new Result instead of the original one.

As I cannot come up with a better name for BindIf, maybe renaming Bind to Transform makes it clearer that it transforms the input whereas BindIf does not.

[vkhorikov]

I think the issue here is the signature of BindIf. It's currently defined as:

public static Result<T, E> BindIf<T, E>(this Result<T, E> result, bool condition, Func<T, Result<T, E>> func)
{
    if (!condition)
    {
        return result;
    }

    return result.Bind(func);
}

Note the same T type arg in the input and output results.

It should be this instead:

public static Result<K, E> BindIf<T, K, E>(this Result<T, E> result, bool condition, Func<T, Result<K, E>> func)
{
    if (!condition)
    {
        return result.Error;
    }

    return result.Bind(func);
}

I believe this overload can replace the one above.

The idea behind BindIf is to just add another condition in addition to the built-in result.IsFailure.

Feel free to submit a PR with this change.

[mnissl]

Your suggestion breaks the semantics of BindIf: the current implementation simply returns the incoming result if the condition is not met. With your suggestion, if the condition is not met, it returns an error. This definitely breaks existing code.

[vkhorikov]

You are right. I didn't consider the behavior when the condition is false.

I don't like the idea of renaming Bind into Transform, though. Bind is an established term from functional programming (F# in particular), this library tries to stick to the existing terminology when possible.

[mnissl]

Well, I'm not familiar with F# and functional programming paradigms in particular. I read a little bit about F# and functional programming yesterday and stumbled over this article on bind where the table of contents also lists in Part 1 other functions like map, return, apply.

Without knowing the concepts of those functions, better names for BindIf could be ReturnIf or ApplyIf ... I don't know whether they clash functionality of different meaning. But at least BindIf sounds wrong to me as it does not transform its input.

[vkhorikov]

But at least BindIf sounds wrong to me as it does not transform its input.

You mean BindIf doesn't change the type of the input? Because it can change the input value itself.

return doesn't change the value, it just wraps it into a monad. As in:

Result<int> result = Result.Success(5);

apply works with functions, not values.

[mnissl]

Well, let me explain the reasoning behind this issue:

I use a chain of mulitple Bind/Tap calls to load data from a database, which basically looks like this:

           db.LoadDataA() .Tap(d => listA.AddRange(d))
.Bind(_ => db.LoadDataB()).Tap(d => listB.AddRange(d))
.Bind(_ => db.LoadDataC()).Tap(d => listC.AddRange(d))
.Finally(r => ConvertToUnitResult(r));

Every new call to Bind does not care about its input, but its output is different from its input, not only data-wise, but also regarding its type.

Recently, I wanted to accomplish an optimization: B needs to be loaded only once, so I thought the following was the way to go:

                            db.LoadDataA() .Tap(d => listA.AddRange(d))
.BindIf(!_listB.Any(), _ => db.LoadDataB()).Tap(d => listB.AddRange(d))
.Bind(                 _ => db.LoadDataC()).Tap(d => listC.AddRange(d))
.Finally(r => ConvertToUnitResult(r));

This didn't work out because now BindIf had to return data type A. So I had to go back to Bind and place the check for listB.Any() inside the Func.

In this regard I say that the naming of BindIf is misleading because BindIf imposes another semantics: you cannot return another data type!

Thus, BindIf is more like a MapIf (i.e. the data type remains the same) ... but Map always returns success if its incoming result is success -- it does not allow for a change of success like Bind and BindIf do.

Does this all make sense to you?

[vkhorikov]

BTW, I like your original code formatting better.

Thus, BindIf is more like a MapIf (i.e. the data type remains the same) ... but Map always returns success if its incoming result is success -- it does not allow for a change of success like Bind and BindIf do.

I assume you meant TapIf and Tap instead of MapIf and Map.

This is by design. The only purpose of Tap is to incur a side effect (e.g save something to a local variable), whereas Bind is for situations where you want to avoid nesting of results as in Result<Result<int>>. Think of SelectMany in LINQ -- it's another example of Bind and helps flatten the collection.

BindIf is different from Bind in that it doesn't change the data type, but it's still similar enough to derive its name from Bind.

Does this all make sense to you?

It does, though I don't know how to solve your issue other than by how you already did it (inline the check into the func). I personally would just use the plain if statements with early exit, but that's probably not the answer you expect.

[mnissl]

My original formatting is how I code in my projects. In order to save vertical space here, I went for this horizontally aligned formatting.

Nope, I did mean Map because Tap does not allow any changes at all: Tap (and TapIf) always return the original result. Map allows for returning a different result, but with the same type and it is always a successful result.

Well, I thought it was a clearer design from a semantics point of view to rename BindIf ... or Bind. Keeping this naming is not a showstopper ... it only caused me some headache why BindIf didn't work and took some time to find out which of the plethora of entension methods might do the trick for me. (In this case unfortunately none.)

One definitely has to have a look at the implementation of the extension methods as you cannot derive the semantics only from its name: will it act on success and/or failure? Will it return a new result or hand through the input result?

I know now what Bind can do and what BindIf can't. I raised this issue only to make life easier for newbies.

The downside of having to put the condition inside the Func is not being able to go for the short lambda expression syntax, but you have to go for the more verbose statement style lambda. After all, the various xxxIfs actually address this issue: shorter, more concise syntax ... for easy and clear reading.