vkhorikov/CSharpFunctionalExtensions

MapIf

FoxTes opened this issue · 11 comments

Hello.
Tell me, please. Why is there no MapIf method?

Unfortunately, my question may seem silly, but alas there is very little documentation.

Most likely because no one needed it so far. New extensions are added in an ad-hoc manner by the community. Feel free to submit a PR.

Alternatively, you can describe your use case (with some code examples), and I will add the extension when I have time.

Thanks for the reply.

I looked at the examples and wrote a method based on them. Unfortunately, I can't create a PR, because I haven't covered all possible options.

public static Result<TK> MapIf<T, TK>(
   this Result<T> result, 
   Func<T, bool> predicate, 
   Func<T, TK> func)
{
   return result.IsSuccess && predicate(result.Value) 
      ? result.Map(func) 
      : Result.Failure<TK>(result.Error);
}

Thanks. Let's keep this ticket open for now, until I (or some one else) implements this method.

Perhaps it is worth opening a PR and gradually filling it?

Is the suggested semantics of MapIf() correct: not satisfying the condition is an error?

BindIf() goes the opposite way: not satisfying the condition returns the same calling result.

The result of Map() is always a successful result if the calling result is successful.

The result of Bind() can be failure even if the calling result is successful.

My feeling for the semantics is that the suggested implementation of MapIf() should have been the implementation of BindIf() ... and vice versa.

See #397 for additional thoughts on this.

Really. Maybe the logic is broken.
Surprisingly, I took an example from the BindIf() method.

Can you give an example of how it should be?

Let's recap the current implementations of Bind(), BindIf() and Map():

public static Result Bind(this Result result, Func<Result> func)
{
  if (result.IsFailure)
    return result;

  return func();
}

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

  return result.Bind(func);
}

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

  return Result.Success(func(result.Value));
}

Bind() and BindIf() allow for turning a successful result into failure. Map() does not. Hence, IMHO, MapIf() should not either.

I'd welcome your implementation, but call it rather TryMap() in order to add the connotation: "This might fail!"

But, for example, there is a method reboot in this variant:

    public static Result<T> BindIf<T>(
        this Result<T> result,
        Func<T, bool> predicate,
        Func<T, Result<T>> func)
    {
        return !result.IsSuccess || !predicate(result.Value) ? result : result.Bind<T, T>(func);
    }

Now I'm completely confused

Where's the confusion? BindIf() returns the same result if it is not successful or if the predicate returns false ... otherwise, func will be executed which allows BindIf() to return failure.

There's actually an issue with the proposed implementation (I'm copying it here):

public static Result<TK> MapIf<T, TK>(
   this Result<T> result, 
   Func<T, bool> predicate, 
   Func<T, TK> func)
{
   return result.IsSuccess && predicate(result.Value) 
      ? result.Map(func) 
      : Result.Failure<TK>(result.Error);
}

The issue comes up when the condition is false but the result is successful. In this case, the result doesn't have an Error and so the method will throw.

We need to look at the most generic implementation of BindIf for an example:

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);
}

Both input and output Results are of the same type T here, and that's to avoid this exact problem. MapIf should be done the same way (i.e. both input and output results should have the same type T), but then it really defeats the purpose of mapping, because the only reason for using Map is to transform T into something else. Probably this is the reason why MapIf wasn't implemented.

Well.
I'd rather wait for someone else to implement this method. If it makes sense, of course.