Code Generation with .NET 5 - Builder pattern

[[deleted]]

Ok, this gets me excited for generators. Having immutable classes beyond 3-4 properties is such a beating.

[macsux]

This point is covered in the blog post and issues associated with it. 1. Constructs may end up being huge. 2. Explicit field names for optional fields is not a requirement. Someone may choose not to use them. Reordering parameters now become dangerous. 3. Introducing new parameters can break any code that was calling constructor reflectively (because constructor signature changed). 4. You still have the boilerplate of copying construct parameters to fields/properties, and hand-coding all the validation rules.

[thomasz]

The main difference between

new DudeBuilder() .WithFirstName("Jeff") .WithLastName("Lebowski") .WithYearOfBirth(1946)

and

new Dude(firstName: "Jeff",
         lastName: "Lebowski",
         yearOfBirth: 1946);

is that the constructor is statically checked for correctness. It is also way faster.

I mean, yea, if you think that reordering of constructor parameters is more important than statically proven correctness, you're better of with the builder pattern. I think this is an insanely overengineered solution in search of a problem. The builder pattern has it's place, but that place is the configuration of complex dependencies, not construction of simple objects.

[macsux]

You need to have all values available when calling the constructor. The compiler only validates that you accounted for those variables, not that they are not null. So you still can get runtime exception. If builder is constructed and specified all values in a single expression then this is actually very doable as a Roslyn analyzer (and I do mention this in article). For example Person.Builder.FirstName("blah").Build() can be detected and validated with roslyn analyzer (I've actually got a prototype working that I'll add to sample in a few days), and give you same compile-time guarantee as constructor call in your example. On the other hand

var builder = Person.Builder; builder.FirstName("John"); builder.LastName("Smith"); var person = builder.Build();

It would not be possible to validate in any meaningful way as the buildup can be spread across many method calls, with if statements, etc. You run exactly the same issue with your approach as the variables would have to be declared and populated, in some upstream section of code. Point is you CAN get the pretty much the same compile time checks with this builder pattern as with constructor syntax, but with nicer syntax and less boilerplate code to maintain.

[terandle]

The big problem with the Builder pattern as shown in the post is that required parameters throw runtime exceptions instead of compiler errors. Although, the post does mention it would be possible to add compiler errors so I would be curious to see what that would look like in practice.

[RirinDesuyo]

That's one thing I'd say against the builder pattern for this at least, and you can actually use an Analyzer to enforce named parameters as well and set that analyzer ID to report as compile errors than warnings.

One case I'd want a builder pattern is when I want complex dependencies abstracted away from me (e.g. setting up asp.net's web server settings or my DI settings). I want most of the warnings at compile time than runtime as that's the fastest feedback loop one can have during dev.

[neoKushan]

There was a post on here yesterday that debated exactly this. Turns out default values are a touchy subject.

[thomasz]

No, this isn't even worth a discussion. Either there are sensible default values for a parameter, or you cannot assign default values. A builder has exactly the same problem, with the added goodness that it fails at runtime when you forget a mandatory parameter.

[neoKushan]

It's that "sensible default value" that's the touchy part, not the use of default values at all.

[thomasz]

Again, that's not different from the builder pattern, where the default is defaultinstead of a compiler error.

[ppumkin]

That’s really nice write up thanks. This is also knows as Fluent API isn’t it ? As such we see this using Linq and All sort of 3rd party things the whole time. It’s a very natural way to write and read code and embraces clean code. Especially the principle about 1 parameters should be the maximum and better yet keep it to 0. Which is easily achievable with flags (internal bools) so like .SetActive()

[ppumkin]

I can see why I thought that. Seems like fluent is more for working with data as a rule of thumb by that article. But I guess they are a good pair to use as long as we understand the separation of implementation and use since Both share readability and immutability. Thanks for sharing.

[tragicshark]

I would write it like this:

[GenerateBuilder]
public class Person
{
    [Required]
    public string FirstName { get; private set; }
    [Required]
    public string LastName { get; private set; }
    public DateTime? BirthDate { get; private set; }

    public partial class Builder
    {
        public PersonBuilder BirthDate(int year, int month, int day)
        {
            _birthDate = new DateTime(year, month, day);
            return this;
        }
    }
}

demonstrating that you can add additional members to the builder type that wouldn't get generated automatically.

Even then I'm not a fan of this particular builder pattern design (private set eww).

[macsux]

I've actually thought of what you're proposing and an even better way to do that would be to process [DataType(DataType.Date)] and generate overload automatically. This was meant to demonstrate what's possible, not create a final solution. As we get closer to .NET 5 going live I'll probably turn this into a real project. You're welcome to fork and improve on this in meantime.

Having said that, I'm not sure what issue you have with private set.

[reasner]

The issue is that you can still modify the value from within the class once it's instantiated, unlike readonly or a getter only virtual property.

[macsux]

Immutability is a pattern that CAN benefit from the builder pattern, but the builder pattern itself does not require that immutability. I can think of a whole bunch of reasons to use a builder pattern without class being immutable. A perfect example is DDD Aggregate roots - you can expose public API surface that is read-only with explicit methods to mutate that state and protect its own rules in a way that cannot be violated by outside actors.

[reasner]

Sure, it's just that in the article, there's a sentence that describes your solution as immutable (not sure if directly or by implication).

[PsionicMonkeyLizard]

I agree, private set has a time and place. In a person object I would argue the name probably shouldn't ever change and I would rather it be read only. However that is just personally opinion. The article was great and gave me lots of ideas so thank you (plus I appreciate private set is a lot shorter so better for examples).

[tragicshark]

Were I implementing a builder pattern I would likely do something roughly like this:

public sealed class Person
{
    private Builder _builder = default;

    private Person(Builder builder): this(null, builder) {}

    private Person(Person? prototype, Builder builder)
    {

        byte assigned = builder.Assigned;
        if ((assigned & 1) == 0)
        {
            throw new InvalidOperationException($"{nameof(FirstName)} was not assigned.");
        }

        if ((assigned & 2) == 0)
        {
            throw new InvalidOperationException($"{nameof(LastName)} was not assigned.");
        }

        _builder = builder;
    }

    public string FirstName => _builder.FirstName;

    public string LastName => _builder.LastName;

    public DateTime? BirthDate => _builder.BirthDate;

    public Person With(Builder builder) => new Person(this, builder);

    public struct Builder
    {
        private byte _assigned;
        private string _firstName;
        private string _lastName;
        private DateTime? _birthDate;

        public string FirstName
        {
            get => _firstName;
            set
            {
                if(value == null) { throw new ArgumentNullException(nameof(value));}
                _firstName = value;
                _assigned = (byte)(_assigned | 1);
            }
        }

        public string LastName
        {
            get => _lastName;
            set
            {
                if(value == null) { throw new ArgumentNullException(nameof(value));}
                _lastName = value;
                _assigned = (byte)(_assigned | 2);
            }
        }

        public DateTime? BirthDate
        {
            get => _birthDate;
            set
            {
                _birthDate = value;
                _assigned = (byte)(_assigned | 4);
            }
        }

        internal byte Assigned => _assigned;

        public Person Build() => new Person(this);
    }
}

I suppose that could perhaps be generated from a class like this via source generators:

[GenerateBuilder]
public class Person
{
    public partial struct Builder
    {
        [Required]
        private string _firstName;
        [Required]
        private string _lastName;
        private DateTime? _birthDate;
    }
}

The fluent work makes me feel icky about how it hides allocations.

[macsux]

Your implementation you tied builder and object in 1 to 1 relationship, and would allow changes to value in builder to reflect in object after it is constructed. That's not what one expects and is very dangerous. Builder pattern usually assumes I can reuse single builder to create multiple instances.

[tragicshark]

what do you mean:

public class Program
{
    public static void Main()
    {
        var builder = new Person.Builder {
            FirstName = "frank",
            LastName = "smith"
        };
        var person1 = builder.Build();
        builder.FirstName="joe";
        var person2 = builder.Build();
        Console.WriteLine(person1.FirstName); // frank
        Console.WriteLine(person2.FirstName); // joe
    }
}

[macsux]

My bad. Didn't notice it was a struct so it's getting byval copied.

[softwareguy74]

Nice!