Conversion of Results

[altmann]

Hi,

I do a lot of conversion of results. My problem is that the following code is not very readable, especially the part Result.Fail<MyValueClass>(result1.Error).

public Result<MyValueClass> DoSomething()
{
   Result result1 = MyMethod();
   if(result1.IsFailure)
      return Result.Fail<MyValueClass>(result1.Error);
   [...]
}

Do you know a simple conversion? If not, I would suggest a new conversion method between results ToResult(). With this conversion method the above code looks like this:

public Result<MyValueClass> DoSomething()
{
   Result result1 = MyMethod();
   if(result1.IsFailure)
      return result1.ToResult<MyValueClass>();
   [...]
}

With ToResult() the already existing implicit operator from Result< T > to Result the conversion between Result and Result< T > is enabled and vice versa.

I only see one open problem with the conversion from an success Result to Result< T >. Result<T> result = Result.Ok().ToResult<T>() Which Value has now variable result? default(T)? Or should this conversion throw an exception?

The conversion from an failed Result to Result< T > makes no problems. Result<T> result = Result.Fail("Failed").ToResult<T>() Value is not set.

What do you think?

BR Michael

[altmann]

I added an PR with the default(T) solution.

[vkhorikov]

Hi Michael,

Is this extension only for situations with failures? I.e when you have a failed result and want to convert it to a failed result of another type?

If so, I think a better extension would be MapFailure(). We already have a similar one but it works on success only (see below). On the contrary, MapFailure would do nothing other than changing the type of the failed result.

    public static Result<K> Map<T, K>(this Result<T> result, Func<T, K> func)
        => result.OnSuccess(func);

Cheers Vlad

[altmann]

Yes, in 99% I need this conversion in situations which I have to convert a failed result to another type of result.

I will have a look to Map(...) and send you an PR with MapFailure(...).

Thanks...

[altmann]

@vkhorikov I have now two new methods but what should I do in the success case?

 public static Result<T> MapFailure<T>(this Result result)
        {
            if (result.IsFailure)
                return Result.Fail<T>(result.Error);

            return ????
        }

        public static Result<K> MapFailure<T, K>(this Result<T> result)
        {
            if (result.IsFailure)
                return Result.Fail<K>(result.Error);

            return ????
        }
```

[vkhorikov]

Throw an exception. This method should be called on a failed result only and if it's not -- it's a bug.

[space-alien]

Just a thought, because when I see...

public Result<MyValueClass> DoSomething()
{
   Result result1 = MyMethod();
   if(result1.IsFailure)
      return Result.Fail<MyValueClass>(result1.Error);
   [...]
}

...I find myself wondering if you couldn't express it thus:

public Result<MyValueClass> DoSomething()
{
   return MyMethod()
      .OnSuccess(() => [...])
      .OnSuccess(() => SomethingThatReturnsMyValueClass())
}

At some point, DoSomething() must presumably create MyValueClass, so with this approach, any failure + error message from MyMethod would have bubbled through correctly, no mapping required.

If a method returns Result, I often find it better for cleanliness/readability to try to avoid checking for failed inner results where possible. For me, this a huge part of the value of using Results.

So I am not too sure about adding a MapFailure() method that depends on checking for a failure before being called.

[altmann]

@space-alien @vkhorikov Our team don't like this railway-orientied approach in some cases. Therefore a MapFailure() method would be very useful to us.

[space-alien]

I understand, but I don't think this changes my mind. Result is all about railway approach, it seems counter-intuitive for the library to add an extension whose purpose is to make it easier to break out of that approach (and usage of which depends on breaking the chain). I think this can make it harder for new users of the library to get the most out of the coding approach it is designed to encourage.

Let's see what Vladimir thinks when he has a chance to catch up with this thread. But perhaps this is something that could be exposed locally in your project as an extension method, rather than added to the library? And we could leave this issue open to see if it gathers more support?

[vkhorikov]

I'm with @altmann on this one. Not all operations can be easily represented with a chain of OnSuccess methods, for example, embedded OnSuccess's are quite hard to grasp and I personally prefer to fall back to regular if-else instead. MapFailure would be really useful in those cases.

[space-alien]

https://github.com/vkhorikov/CSharpFunctionalExtensions/blob/1bcb4674ba72667acdfd5d98d3822120f453e1e3/CSharpFunctionalExtensions/Result/Methods/ConvertFailure.cs#L18

public partial struct Result<T>
{
    public Result ConvertFailure()
    {
        if (IsSuccess)
            throw new InvalidOperationException(Result.Messages.ConvertFailureExceptionOnSuccess);

        return Result.Failure(Error);
    }
}

The above signature seems redundant, as the same could be achieved with a simple cast: (Result)myResult

Propose removing this signature?

[vkhorikov]

I agree, no need for this one 👍