Factory Methods

[mumby0168]

I was thinking it might be quite nice to have an opt-in feature that makes you rely on factory methods to create the different types within a union. I was thinking this could be another attribute maybe [UnionWithFactory] or something like [Union(factory: true)].

I took the code example from the samples and modified it to show how the factory methods could be used:

using Dunet.Choices;
using Dunet.Shapes;

var rectangle = Shape.Rectangle(10, 10);
var triangle =  Shape.Triangle(10, 10);
var circle = Shape.Circle(10);

var getArea = (IShape shape) =>
    shape switch
    {
        Rectangle rect => rect.Length * rect.Width,
        Circle circle => 2.0 * Math.PI * circle.Radius,
        Triangle triangle => 1.0 / 2.0 * triangle.Base * triangle.Height,
        _ => 0d,
    };

var rectangleArea = getArea(rectangle);
var triangleArea = getArea(triangle);
var circleArea = getArea(circle);

Console.WriteLine($"Rectangle area: {rectangleArea}");
Console.WriteLine($"Triangle area: {triangleArea}");
Console.WriteLine($"Circle area: {circleArea}");

var choice = GetChoice();

if (choice is Yes)
{
    Console.WriteLine("YES!!!");
}

if (choice is No)
{
    Console.WriteLine("NO!!!");
}

static IChoice GetChoice() =>
    Console.ReadLine() switch
    {
        "yes" => Option.Yes(),
        _ => Option.No()
    };

What do you think?

[domn1995]

I think this is a great idea. It would also work nicely when we add generic support since we cannot infer generic types with constructors.

[domn1995]

I think #17 addresses this use case with a more superior implementation due to more control over the union members' types. What do you think @mumby0168 ?

[mumby0168]

I think #17 addresses this use case with a more superior implementation due to more control over the union members' types. What do you think @mumby0168 ?

Hi yes I like this, no reason the root record, the Result in the example cannot contain factory methods also. I.e Results.Ok() with the need of the new keyword, it could follow the same pattern with a bool to indicate you want the extra methods generating.

What do you think?

[domn1995]

Hi yes I like this, no reason the root record, the Result in the example cannot contain factory methods also. I.e Results.Ok() with the need of the new keyword, it could follow the same pattern with a bool to indicate you want the extra methods generating.

What do you think?

I'm having a hard time seeing a generated factory method like Result.Ok() playing nicely with #17 also providing a way to instantiate that same union member with new Result.Ok() or Result.NewOk() due to name collisions. I'm inclined to prioritize the implementation of #17, and then reassessing how we feel about this issue after that.

[saul]

Preliminary PR for this here: #33

[domn1995]

After some more analysis, I'm going to defer this feature for the following reasons:

• The ability of users to write Foo.NewThing() rather than new Foo.Thing() is not worth the increase in complexity and potential maintenance burden to this library.
• Source generators already have a steep learning curve, due to a lot of hidden/non-explicit behavior. I believe a successful source generator should do as much as it can to minimize such behavior and take an explicit approach whenever possible. This feature runs counter to that philosophy.
• With Implicit conversions #28 on the docket, constructors and factory methods will not be necessary for most use cases. This is because users will be able to assign inner union values directly to the union type. This will bring Dunet unions in line with most other language's DU implementations and effectively obsolete constructors and factory methods. For example:

// No need for constructor or factory methods to instantiate union values.
StringOrNumber str = "foo";
StringOrNumber num = 123;

[Union]
partial record StringOrNumber
{
    partial record Str(string Value);
    partial record Num(double Value);
}

If the community remains interested in this feature, I would whole-heartedly support a separate package containing this functionality. I'd also be glad to link to it from Dunet's readme if that becomes the case.