vkhorikov/CSharpFunctionalExtensions

EnsureNotNull

hankovich opened this issue · 3 comments

In our project we quite heavily use

Result<T, TError> EnsureNotNull<T, TError>(this Result<T?, TError>> result, TError error) where T : struct;
Result<T, TError> EnsureNotNull<T, TError>(this Result<T?, TError>> result, TError error) where T : class;

and I'm curious does it makes sense to have them (with all overloads of course) in CSharpFunctionalExtensions.
I'm happy to see them here, but we don't have nullable reference types enabled. But we can wrap extensions in #nullable enable.

Given that the library makes extensive use of Maybe, is it possible to have these extensions as following?

Result<T, TError> EnsureNotNull<T, TError>(this Result<Maybe<T>, TError>> result, TError error) where T : struct;

There might not be an implicit conversion from Result<T, TError> to Result<Maybe<T>, TError> but I think we can add it to the library, so that for the clients both your and this versions would behave the same way.

@vkhorikov I found that

  1. Implicit convertion you've mentioned is not applied automatically
// with added implcit convertion
// with public static Result<T> EnsureNotNull<T>(this Result<Maybe<T>> result, string error)

Result<T?> result = Result.Failure<T?>(ErrorMessage);

Result<T> returned = result.EnsureNotNull(ErrorMessage2); // cannot resolve EnsureNotNull
Result<T> returned = ((Result<Maybe<T>) result).EnsureNotNull(ErrorMessage2); // works fine
  1. Any convertion cannot be applied to Task<T> since classes doesn't support covariance and contravariance

So overloads for Maybes will not cover overloads for nullable reference types.
And tbh with NRT enabled we don't use Maybe at all

OK, merging the PR as-is then.