vkhorikov/CSharpFunctionalExtensions

Bind() overloads missing, causing method resolution errors.

Closed this issue · 5 comments

The problem

There are no overloads for Bind() that will bind a Result to Result<T, E>. It's not intuitive that this is a problem, and the error message that the compiler gives is not very helpful.

For example

Result.Success("foobar") // Result<string>
      .Bind(input => GetStrings(input)); // build fails with error "CS0029 Cannot implicitly convert type 'CSharpFunctionalExtensions.Result<System.Collections.Generic.IEnumerable<string>, UserQuery.MyErrorClass>' to 'System.Threading.Tasks.Task<CSharpFunctionalExtensions.Result>'"

Result<IEnumerable<string>, MyErrorClass> GetStrings(string input)
    => Result.Success<IEnumerable<string>, MyError>(new [] { input });

public record MyError(string message);

Workaround

The workaround for this is to convert the Result<T> to a Result<T, E> before calling Bind.

Result.Success("foobar") // Result<string>
      .MapError(e => new MyError(e)) // Result<string, MyError>
      .Bind(input => GetStrings(input)); // Result<IEnumerable<string>, MyError>

Solution

My proposed solution is to add overloads for Bind that accept Result<T> and return Result<T, E>.

The method signature public static Result Bind<T, K, E>(this Result<T> result, Func<T, Result<K, E> func) illustrates the solution. Similar overloads will be needed to support tasks as well.

I'm happy to contribute a PR for this if desired.

Yep, sounds good. If it doesn't introduce any conflicts signature wise, let's add these overloads. Please proceed with a PR.

I started on a PR, but I'm thinking that this might not be possible. The type Result<T> has an implicit error type of string, and there isn't a way to convert string to E.

public static Result<K, E> Bind<T, K, E>(this Result<T> result, Func<T, Result<K, E>> func)
{
    if (result.IsFailure)
        return Result.Failure<K, E>(result.Error); // This causes a build error of "Argument type 'string' is not assignable to parameter type 'E'"

    return func(result.Value);
}

One option would be to include a second predicate Func to convert the error to E. I'm not sure if that would add extra complexity, especially when calling MapError as I suggested in my workaround above is a viable option. One solution is to add a second Func<string, E> parameter to the method to allow that mapError behavior to be injected:

public static Result<K, E> Bind<T, K, E>(this Result<T> result, Func<T, Result<K, E>> func, Func<string, E> mapError)
{
    if (result.IsFailure)
        return Result.Failure<K, E>(mapError(result.Error));

    return func(result.Value);
}

I'm torn between just abandoning this and adding that second Func parameter. I feel like adding it would make the conversion between Result<T> and Result<K, E> more explicit and obvious, but is there a better path forward?

Your Workaround code looks good to me. I think it is the right way for the scenario.

It looks like those overloads were missed exactly because of the issue you've mentioned.

I agree with the solution and I think it's better to name the latest parameter errorFactory, but it's a matter of taste.

@rutkowskit I agree, it's a workable solution.

@hankovich After diving into the code, I agree. However, I doubt it's necessary to implement such a method given a viable alternative.

I'm going to close this, but feel free to reopen it if you feel it appropriate to implement this.