AKlaus / DomainResult

Tiny package for decoupling domain operation results from IActionResult and IResult types of ASP.NET Web API
Apache License 2.0
53 stars 3 forks source link

Add Implicit operator to Convert to IDomainResult<T> from DomainResult #54

Closed RHaughton closed 1 year ago

RHaughton commented 2 years ago

Hi,

I would like to see an implicit converter to IDomainResult from DomainResult.

When a function is returning a Task<IDomainResult>, I would like to be able to return an IDomainResult and automatically cast it to IDomainResult.

RHaughton commented 2 years ago

It would look something like that:

public class DomainResult<T>
{
    // Existing code...

    public static implicit operator IDomainResult<T>(DomainResult result)
    {
        if (result.IsSuccess) {
            throw new InvalidOperationException("Cannot implicitly cast a successful DomainResult");
        }

        return From(result);
    }
}

this would allow to directly return and IDomainResult without using generics.

this: return DomainResult<T>.From(DomainResult.NotFound("msg")); or return DomainResult.NotFound<T>("msg"); to this return DomainResult.NotFound("msg");

AKlaus commented 2 years ago

Thanks mate for your suggestion 👍

At first glance, my only objection would be coming from a non-obvious C# limitation where the implicit conventions won't work for interfaces. E.g.

public static implicit operator DomainResult<TValue>(IDomainResult domainResult) => ...

wouldn't compile due to [CS0552] 'DomainResult<TValue>.implicit operator DomainResult<TValue>(IDomainResult)': user-defined conversions to or from an interface are not allowed. See this discussion on the language limitation – dotnet/csharplang/discussions/3464 (please upvote if it makes sense).

So if a dev passes around the domainResult as DomainResult it works and if it happens to be passed as IDomainResult then it won't compile puzzling the dev.

Regarding the linked PR, I have another minor one that it leads to a runtime error if a dev forgets to check IsSuccess prior to an implicit conversion. Though, it can be easily mitigated (posted there).

AKlaus commented 1 year ago

Verdict

Sorry mate, in spite me really liking the proposal, I have to decline it on the grounds that

  1. It would introduce a disparity between DomainResult<T> and IDomainResult<T> helper methods.
  2. Can't convert IDomainResult to either DomainResult<T> or IDomainResult<T> (see my comment above), and passing interfaces around is more prevalent.

Recommended use

For the current case of converting IDomainResult domainResult to IDomainResult<T> type, please use one of the available explicit conversions:

  1. domainResult.To<T>() – the shortest one
  2. DomainResult<T>.From(domainResult)

Technicalities:

Due to the already mentioned C# lang limitation (dotnet/csharplang/discussions/3464), while the following implicit operator for a class would be legit:

static implicit operator DomainResult<T>(DomainResult domainResult) => new (domainResult);

two similar ones for an interface fail compilation with "user-defined conversions to or from an interface are not allowed" error:

static implicit operator DomainResult<TValue>(IDomainResult domainResult) => new (domainResult);```
and
```csharp
static implicit operator IDomainResult<T>(DomainResult domainResult) => DomainResult<T>.From(domainResult);

I hoped that C# 11 would bring a change, but nope, still same old. This StackOverflow post has good suggestions from the C# gurus on why it's been implemented this way.

The recommended way for type conversion from the MS guys is to use an explicit ToXXX extension method, like ToArray, ToList, ToHashSet, but it wouldn't fit the case as we need a generic method like

static IDomainResult<T> ToDomainResult<T>(this DomainResult domainResult)

that, again, won't pass the compiler due to "Extension method can only be declared in non-generic, non-nested static class" error 🤦.