Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Buildable Immutable Types #9830

Closed
andrewjsaid opened this issue Mar 17, 2016 · 5 comments
Closed

Proposal: Buildable Immutable Types #9830

andrewjsaid opened this issue Mar 17, 2016 · 5 comments

Comments

@andrewjsaid
Copy link

Like the rest of you, I try to create immutable types where possible. Consider the simple Person class below:

public class Person
{
    public Person(string name)
    {
        Name = name;
        // ... set other properties
    }

    public string Name { get; }
    // ... define properties
}

As the number of fields grow, the parameters start to get quite out of control. Now consider that the properties of Person must be calculated from a variety of sources. In our build method we can either have variables all over the place, or we must create a new PersonBuilder class which stores properties and instantiates the Person class.

// Creates a person based on some fancy logic
public Person CreatePersonUsingVariables(...)
{
    var name = ...;

    // complex computations of various properties
    // this logic can have branches which set different properties
    // or might call to methods.

    return new Person(name, ...);
}


// Creates a person based on a builder class
public Person CreatePersonUsingBuilder(...)
{
    var builder = new PersonBuilder();

    builder.Name = ...;
    // set properties when needed, can pass builder around to other methods

    return builder.Build();
}

// Builder (have to define manually)
public class PersonBuilder
{
    public string Name { get; set; }
    // ... define properties

    public Person Build() => new Person(name, ...);
}

The drawbacks of creating your own builder class are related to maintainability and overhead. Many times developers will just forego immutability in order to save time having to defines these builder types.

My proposal is for C# to automatically create a builder class. Syntax would look something like this:

// C# code
public buildable class Person
{
    string Name { get; }
}

// Compiler generates
public class Person
{
    public Person(string name)
    {
        Name = name;
        // ... set other properties
    }

    public string Name { get; }
    // ... define properties

    public class Builder
    {
        public string Name { get; set; }
        // ... define properties

        public Person Build() => new Person(name, ...);
    }
}

Things to note:

  • The modifier buildable lets the compiler know how to treat this.

  • Properties on buildable types are implicitly public. Like interfaces no modifier is allowed.

  • The compiler adds a public nested class within the buildable type called "Builder".

  • In all other ways the compiler can treat Person and Person.Builder as regular types.

  • We can add methods to the Person type. If methods on Person.Builder are needed, the following syntax can be used:

    // C# code
    public buildable class Person
    {
        string Name { get; }
    
        // Methods applicable to Person
        public void Eat()
        { 
            // In this scope, properties are readonly
        }
    
        // I can choose to override GetHashCode() and Equals(object)
    
        builder // Defines a scope for methods of Person.Builder
        {
            public void CalculateAge(DateTime dob) {
                // In this scope, properties are mutable
            }
        }
    }
    
  • We can also have a method which generates the builder from the type.

    // Compiler generated, for brevity I have excluded code from previous listing
    public class Person
    {       
        public Builder GetBuilder()
        {
            return new Builder
            {
                Name = name,
                // ... other properties
            };
        }
    }
    
  • Optional: We can extend this to buildable structs

@andrewjsaid andrewjsaid changed the title Buildable Immutable Types Proposal: Buildable Immutable Types Mar 17, 2016
@HaloFour
Copy link

There seems to be a bit of crossover between this proposal and records/withers (#206, #5172). The entire concept could probably be implemented through code-generation (#5561) and a custom attribute rather than making it a proper extension to the language.

@andrewjsaid
Copy link
Author

Thank you for your consideration and reply.

I agree that this concept could probably be implemented through code-generation and custom attributes (#5561). I think it could probably also be implemented through T4 templates. However I proposed it as a language feature because:

  • It is in line with the direction which C# seems to be headed.
  • It is a common across-the-board scenario which reduces boilerplate code.
  • Using buildable can have semantic meaning in certain use cases (has overlaps with the with operator (Proposal: "with" expressions for record types #5172) as you yourself state).
  • A language which supports both immutability and mutability should have an easy way of switching between the two.
  • The syntax I proposed seems to fit with C#.

Although I acknowledge some common goals/principles, my proposal differs from records (#206) because my focus/motivation is on making construction of these objects easier, rather than having an easy way to define immutable types. Whilsts records fully embrace immutability we must keep in mind that lots of C# code does modify state. Often it is only at the point when complex operations are finished when the immutable objects are created. Records as they are, are great when all the data for the record is available, but this is usually not the case for larger operations.

The advantage of the buildable modifier as I have suggested it is that a class/task/method can take advantage of state when needed but quickly and without punishment provide immutable results for the remaining pipeline of work with.

To be clear, my proposal may just as well be absorbed as another property of record types, as the two are not mutually exclusive; I am just focussing on construction rather than declaration.

It is also easy to see how the with operator (#5172) can complement this proposal; the same syntax can be used too. I don't think there will be any problems with position-to-property matching identified in #9330.

@andrewjsaid
Copy link
Author

I found a blog post in which Andrew Arnott describes the problem this issue addresses. He goes on to solve it using the T4 template route and provides many more features than suggested in my original post. There are many similarities in the code I suggested and in his solution, which to me indicates that it is an intuitive one.

The blog post is here:
https://blogs.msdn.microsoft.com/andrewarnottms/2013/01/08/simple-immutable-objects/

The repository can be found here:
https://github.com/AArnott/ImmutableObjectGraph

I have chosen to leave this issue open as I still believe that C# should have support for switching between mutable and immutable objects without relying on a third party solution, and for the reasons I mentioned in my previous comment.

@Joe4evr
Copy link

Joe4evr commented Apr 22, 2016

With the fact that code generation is coming directly into Roslyn, I'm not sure if a new keyword would be the ideal solution for this. You could just write a generator looking for an attribute like [Buildable] with no need for T4 either.

//Person.cs
[Buildable]
public partial class Person //might not even need 'partial' here if partial partials will be a thing
{
    public string FirstName { get; }
    public string LastName { get; }
}

//Person.g.cs
//automatically expands whenever new properties are added to the other partial
public partial class Person
{
    public Person(Builder builder)
    {
        FirstName = builder.FirstName;
        LastName = builder.LastName;
    }

    public class Builder
    {
        public string FirstName { get; set; }
        public string LastName { get; set; }
    }
}

@gafter
Copy link
Member

gafter commented Mar 24, 2017

We are now taking language feature discussion in other repositories:

Features that are under active design or development, or which are "championed" by someone on the language design team, have already been moved either as issues or as checked-in design documents. For example, the proposal in this repo "Proposal: Partial interface implementation a.k.a. Traits" (issue 16139 and a few other issues that request the same thing) are now tracked by the language team at issue 52 in https://github.com/dotnet/csharplang/issues, and there is a draft spec at https://github.com/dotnet/csharplang/blob/master/proposals/default-interface-methods.md and further discussion at issue 288 in https://github.com/dotnet/csharplang/issues. Prototyping of the compiler portion of language features is still tracked here; see, for example, https://github.com/dotnet/roslyn/tree/features/DefaultInterfaceImplementation and issue 17952.

In order to facilitate that transition, we have started closing language design discussions from the roslyn repo with a note briefly explaining why. When we are aware of an existing discussion for the feature already in the new repo, we are adding a link to that. But we're not adding new issues to the new repos for existing discussions in this repo that the language design team does not currently envision taking on. Our intent is to eventually close the language design issues in the Roslyn repo and encourage discussion in one of the new repos instead.

Our intent is not to shut down discussion on language design - you can still continue discussion on the closed issues if you want - but rather we would like to encourage people to move discussion to where we are more likely to be paying attention (the new repo), or to abandon discussions that are no longer of interest to you.

If you happen to notice that one of the closed issues has a relevant issue in the new repo, and we have not added a link to the new issue, we would appreciate you providing a link from the old to the new discussion. That way people who are still interested in the discussion can start paying attention to the new issue.

Also, we'd welcome any ideas you might have on how we could better manage the transition. Comments and discussion about closing and/or moving issues should be directed to #18002. Comments and discussion about this issue can take place here or on an issue in the relevant repo.

I am not moving this particular issue because I don't have confidence that the LDM would likely consider doing this as proposed. Most of the value of this proposal would be available by writing a record declaration and using the record type itself as your builder.

@gafter gafter closed this as completed Mar 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants