Proposal: Anonymous discriminated unions

[gafter]

@LarsKemmann commented on Fri Feb 20 2015

Scenario: I'm currently factoring out the entity manipulation code from a web application into a standalone library for reuse and easier maintenance. A lot of it takes the following form (totally mangling the details):

[HttpPost]
public ActionResult CreateFoo(CreateFooArguments args)
{
    if (ArgumentsAreValid(args))
    {
        using (var context = SomeDbContext())
        {
            var userInfo = GetUserInfo(context, this.UserId);

            if (UserHasAppropriateAccess(userInfo))
            {
                var newFooId = DoWorkToCreateFoo(userInfo, model.name);
                return new JsonResult(newFooId);
            }
            else
            {
                return new HttpStatusCodeResult(HttpStatusCode.NotAllowed, "The user does not have access.");
            }
        }
    }
    else
    {
        return new HttpStatusCodeResult(HttpStatusCode.BadRequest);
    }
}

To factor out the entity creation along with the validation requires that the CreateFoo method have a way to map multiple return types into the appropriate HttpStatusCodeResult or JsonResult. Anonymous discriminated unions would be the ideal solution for this, since I get the convenience of having everything defined as part of the method.

[HttpPost]
public ActionResult CreateFoo(CreateFooArguments args)
{
    switch (ExternalCreateFoo(args.name, this.UserId))
    {
        case FooCreated(int id):
            return new JsonResult(id);
        case UserHasNoAccess:
            return new HttpStatusCodeResult(HttpStatusCode.NotAllowed, "The user does not have access.");
        case ArgumentsAreInvalid:
            return new HttpStatusCodeResult(HttpStatusCode.BadRequest);
    }
}

public FooCreated(int id) | UserHasNoAccess | ArgumentsAreInvalid ExternalCreateFoo(string name, int userId)
{
    if (ArgumentsAreValid(args))
    {
        using (var context = SomeDbContext())
        {
            var userInfo = GetUserInfo(context, userId);

            if (UserHasAppropriateAccess(userInfo))
            {
                var newFooId = DoWorkToCreateFoo(userInfo, model.name);
                return FooCreated(newFooId);
            }
            else
            {
                return UserHasNoAccess;
            }
        }
    }
    else
    {
        return ArgumentsAreInvalid;
    }
}

---

@LarsKemmann commented on Fri Feb 20 2015

And I apologize in advance for not using correct terminology. I've built one compiler in my life and it only dealt with integers. :)

---

@svick commented on Sat Feb 21 2015

So, you're basically asking for anonymous discriminated unions?

Also, why should such type be available only as a method return type, and not also everywhere else where a normal type can appear?

---

@LarsKemmann commented on Sat Feb 21 2015

Yes, exactly!

They could certainly be available more generally. The method return type was the scenario that seemed most useful when I wrote this up.

---

@dsaf commented on Mon Feb 23 2015

I can easily imagine F# doing this if it did not (over-)use type inference.

BTW it also sounds a bit like http://en.wikipedia.org/wiki/Postcondition

---

@Richiban commented on Fri Mar 06 2015

What you've described is very very close to F#'s active patterns. I rewrote your example in F#, tell me if it's close to what you wanted in readability, then maybe the feature can be ported over!

    let (| FooCreated | UserHasNoAccess | ArgumentsAreInvalid |) (name : string, userId : int) =
        if ArgumentsAreValid (name, userId)
        then
            use context = SomeDbContext()
            let userInfo = GetUserInfo(context, userId);

            if UserHasAppropriateAccess userInfo
            then
                let newFooId = DoWorkToCreateFoo (userInfo, name)
                FooCreated newFooId
            else
                UserHasNoAccess
        else
            ArgumentsAreInvalid


    [HttpPost]
    member this.CreateFoo (args : CreateFooArguments) =

        match args.name, this.UserId with
        | FooCreated id ->
                JsonResult id
        | UserHasNoAccess ->
                HttpStatusCodeResult (HttpStatusCode.NotAllowed, "The user does not have access.")
        | ArgumentsAreInvalid ->
                HttpStatusCodeResult HttpStatusCode.BadRequest

---

@mcintyre321 commented on Thu Mar 31 2016

I (like several others) have made a library for doing this, called OneOf which achieves this using implicit operators e.g.

public OneOf<User, InvalidName, NameTaken> CreateUser(string username)
{
    ...
}

I would love this to be a language feature though, as OneOf has some limitations (e.g. implicit operators don't work on interface types)

[LarsKemmann]

I would still love to see this -- and yes, @Richiban's F# example probably comes pretty close to it as does @mcintyre321's.

[jnm2]

Wouldn't this be part of #399?

[mcintyre321]

Is there a need for #399's intersection types in C#? We're quite well stocked with ways to group values together with ValueTuple and anonymous objects.

[jnm2]

I'd use both intersection and union types. It comes up a lot, especially in discussions on things like catch (FooException | BarException ex).

[mcintyre321]

That's a union, not an intersection i think

[jnm2]

Yes, I would use intersections too.

if (obj is Foo & IBar fooBar)
{
    fooBar.FooMethod();
    fooBar.BarMethod();
    LegacyConstrainedGenericMethod(fooBar);
}

void LegacyConstrainedGenericMethod<T>(T fooBar) where T : Foo, IBar
{
    // ...
}

[mcintyre321]

I'm sure that might look more normal after I've looked at it a bit longer, but I accept it's a kind of sugar!

Unions, which enable exhaustive switch/matching, enforce that developers handle new cases at compile time. For me that's a killer feature.

I wouldn't want this proposal (to add Unions) to be joined at the hip to the intersections proposal, which I suspect will be more controversial as there are similar language features and workarounds already. I agree that it's a subset of #399 though.

[jjvanzon]

I understand why you would want this. I would use it in a few places where otherwise the common type would be object, and I would leave the caller guessing which types of things it could return, or put it in the member's summary, which someone might forget to update if something changes. Or make an awkward tuple-like type of which only one of the three properties actually holds a value. However, for the use case you presented I have a different solution. My solution was to return the type that actually holds the result in case of success, and let the other two cases be thrown as exceptions to be handled by central error handling in the global.asax, redirecting it to the right error page. Not only did this solve the problem that the return type was object, it made security violations safer, since it became harder to accidently ignore the security violation result, since it was thrown as an exception. The other case, where you say, that if data is invalid, then it is a bad request, may also be better off with throwing an exception. I do not say handle everything with an exception, but this bad request case should be indeed a very exceptional case.

I am saying a different example may make a better case. But I agree it would be useful.

[LarsKemmann]

Definitely a fair point. However, a global.asax may not be an appropriate way to return an error in all cases. For instance, in a Web API, I want to return different types of HTTP status codes for different types of issues (validation is a 400, missing object is a 404, operation problems are a 503, etc.). Differentiating those at compile time in an interface on the model layer in an MVC architecture, e.g., would be really helpful. ~Lars From: Jan-Joost van Zon<mailto:[email protected]> Sent: Friday, April 21, 2017 1:06 PM To: dotnet/csharplang<mailto:[email protected]> Cc: Lars Kemmann<mailto:[email protected]>; Mention<mailto:[email protected]> Subject: Re: [dotnet/csharplang] Proposal: Anonymous discriminated unions (#468) I understand why you would want this. I would use it in a few places where otherwise the common type would be object, and I would leave the caller guessing which types of things it could return, or put it in the member's summary, which someone might forget to update if something changes. Or make an awkward tuple-like type of which only one of the three properties actually holds a value. However, for the use you presented I have a different solution. My solution was to return the type that actually holds the result in case of success, and let the other two cases be thrown as exceptions to be handled by central error handling in the global.asax, redirecting it to the right error page. Not only did this solve the problem that the return type was object, it made security violations safer, since it became harder to accidently ignore the security violation result, since it was thrown as an exception. The other case, where you say, that if data is invalid, then it is a bad request, may also be better off with throwing an exception. I do not say handle everything with an exception, but this bad request case should be indeed a very exceptional case. I am saying a different example may make a better case. But I agree it would be useful. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://github.com/dotnet/csharplang/issues/468#issuecomment-296248102>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AFkGvRxUufxCnQ-eNh7CmFnoAHoP7x1sks5ryOH2gaJpZM4NDvoP>.

[jjvanzon]

@LarsKemmann
The solution to your use case I proposed was indeed implemented in an MVC front-end app, but I didn't only redirect to a page in my central exception handling, I was also able to set the HTTP status code. I have not built such a solution with Web API, but I am quite convinced I would in that case also find a central place in which to handle it and use a similar pattern, because of the aforementioned benefits (security, no type ambiguity, satisfying the DRY principle). Anyway, perhaps I am wrong about Web API.

Here is some pseudo code of the use case that I had, which is similar to your own. The class is like a platform-indendent version of an MVC controller. The Confirm method will return a different view model depending on the situation.

public class ConfirmDeletePresenter
{
    /// <summary> Can return LoginViewModel, NotFoundViewModel or DeleteConfirmedPresenter. </summary>
    public object Confirm(int id)
    {
        if (string.IsNullOrEmpty(_authenticatedUserName))
        {
            var presenter2 = new LoginPresenter(_repositories);
            LoginViewModel loginViewModel = presenter2.Show();
            return loginViewModel;
        }

        Question question = _repositories.QuestionRepository.TryGet(id);
        if (question == null)
        {
            var presenter2 = new NotFoundPresenter(_repositories.UserRepository, _authenticatedUserName);
            NotFoundViewModel notFoundViewModel = presenter2.Show();
            return notFoundViewModel;
        }

        // Deletion logic...

        var presenter3 = new DeleteConfirmedPresenter(_repositories, _authenticatedUserName);
        DeleteConfirmedViewModel deleteConfirmedViewModel = presenter3.Show(id);
        return deleteConfirmedViewModel;
    }
}

The union type in your proposal would neatly replace the type 'object' that the method returns.

I was trying to find a better example where it would be useful. However, as I stated earlier, later I threw a NotFoundException and UnauthorizedAccessException and AuthenticationException, handled those centrally and the need for returning 'object' almost vanished from this part of my code base(s).

But in cases where I don't have a different solution, it would be awesome.

[mcintyre321]

Discriminated Unions CV as return types on are useful all the way through your codebase, not just at the controller level, as they give you a compiler error, requiring a specific handler when new cases are added, rather than relying on a generic catch all.

[LarsKemmann]

I agree with @mcintyre321 -- using exceptions for control flow is an anti-pattern. The .NET Framework performance rules include this as well:

While the use of exception handlers to catch errors and other events that disrupt program execution is a good practice, the use of exception handler as part of the regular program execution logic can be expensive and should be avoided. In most cases, exceptions should be used only for circumstances that occur infrequently and are not expected.. Exceptions should not be used to return values as part of the typical program flow. In many cases, you can avoid raising exceptions by validating values and using conditional logic to halt the execution of statements that cause the problem.
Source: MSDN

@janjoostvanzon The scenario of an unauthorized user is not exceptional. It should be considered a normal part of the work done by the system and thus represented explicitly. Exceptions are a poor fit for this both because they are invisible to the caller and because throwing an exception results in a lot of unnecessary steps being taken (such as a stack walk). Moreover, in the worst (and quite likely) case of an exception being unhandled, the exception will take down the entire process. Yes, ASP.NET MVC has something of a workaround for this by catching exceptions thrown in requests and returning a 503 error, but that's not a universal solution. I'm working on several long-running services that need proper flow control which is best represented using discriminated unions (or something akin to them, I only know a little bit about language design :)).

[jjvanzon]

@LarsKemmann
I agree in general with you about exceptions not being meant for control flow. See my rant on #464

But it is new to me that a tiny exception will take down my whole site. Second of all, a tiny programming error made by a (junior?) developer putting a huge hole in my security, is prevented by the choice to use exceptions for this. If there is a problem with security I want everything to stop and go straight to the main error handler. I feel I made a pretty good trade-off here.

[mcintyre321]

This delve into exception handling is bit of a derailment of the original proposal - if you are wondering about the value of Discriminated Unions and what they bring to the table, have a read of the google results for F# Discriminated Unions, or the docs page for OneOf

[sdedalus]

I did a simple implementation of an anonymous discriminated unions a year or two back including an exhaustive match syntax, from the perspective of a developer in the language vs a language developer the actual Union type is not a problem it's a generic with implicit casts.

Getting a value out is where it gets tricky I went with a fluent syntax for pattern matching because there is no way to leverage C# pattern matching in a strict manor in this scenario. The only way to leverage C# pattern matching is to cast to a common sub type which defeats the point of having a discriminated union in the first place. My point here is this if the integration with pattern matching can be worked out the union type itself should be simple. That may be as simple as adding an Option/Maybe type and exposing a Deconstruct operation on the discriminated union base such that Union<A, B, C>
deconstructs to (Option<A>, Option<B>, Option<C>) and the special case of Value Tuple of Option can be transformed in pattern matching.

var someString = myUnion switch
{
  A _ => "It's A",
  B _ => "It's B",
  C _ => "It's C",
  null, _ => "None"
};

gets transformed to

var someString = myUnion switch
{
  (Some, _, _) => "It's A",
  (_, Some, _) => "It's B",
  (_, _, Some) => "It's C",
  null, _ => "None"
};

https://github.com/sdedalus/DiscriminatedUnion

[Joe4evr]

@sdedalus Please fix your markdown. It's really hard to take someone serious if their post is mis-formatted.

[jnm2]

It's really hard to take someone serious

s/serious/seriously 😜

[mcintyre321]

@sdedalus maybe you can do what you are trying by using OneOf as it doesn't use fluent syntax?

[sdedalus]

@mcintyre321 OneOf seems like a similar library to what I did we should compare notes and collaborate.

In this instance I was less concerned with the actual library as using my experiences to illustrate the point that you can achieve something close to Anonymous Discriminated Unions without modifications to the language but the pattern matching syntax is not yet optimal for unpacking them. In the process I proposed a syntax rewriting approach using existing language features to support unpacking Discriminated Unions. I assume the developers of the BCL can outdo us both as far as an actual Discriminated Union and Option/Maybe implementation goes.