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

Implement property initializers for immutable types #229

Closed
dsaf opened this issue Feb 4, 2015 · 38 comments
Closed

Implement property initializers for immutable types #229

dsaf opened this issue Feb 4, 2015 · 38 comments

Comments

@dsaf
Copy link

dsaf commented Feb 4, 2015

Immutable types are a best practice nowadays. However, when a C# class has a lot of properties it means ctor will have lots of parameters. Property initializers are easier to read and understand as well.
Can we have a way of initializing properties on immutable types? I have read about C# 6 auto-property initializers and unless I am mistaken they only help with compile-time constants.

PS: please don't do this by introducing new types (e.g. why have some sort of 'record' when we already have classes and structs?).

@SolalPirelli
Copy link

You can already assign values to get-only properties from the constructor in C# 6. (source)

@dsaf
Copy link
Author

dsaf commented Feb 4, 2015

@SolalPirelli

This is not what I am asking. I want this:

var student = new Student(new Guid())
{
   Name = "John",
   Address = GetAddressFromDb(),
   Age = minAge + 20
}

All properties in the sample above are immutable (readonly).

@AlgorithmsAreCool
Copy link

So you would like to see object initializers gain the ability to write to getter only properties? Like this?

class Foo
{
    public int Bar { get; }
}
...

Foo myFoo = new Foo() { 
    Bar = 42
};

@dsaf
Copy link
Author

dsaf commented Feb 4, 2015

@AlgorithmsAreCool

Yes, exactly. Otherwise the number of constructor parameters is starting to affect the choice of whether to make something immutable or not. Which is not great at all.

Furthermore, it would need to be reflection-friendly to make sure deserialisation (e.g. from JSON) is still possible.

@dsaf
Copy link
Author

dsaf commented Feb 4, 2015

@dsaf
Copy link
Author

dsaf commented Feb 4, 2015

Maybe 'readonly' keyword could be re-used to mark the whole type:

 //All properties and fields have to be declared as readonly or const and be initialised explicitly.
readonly class Student { ... }

This post shares this idea as well:

http://madprops.org/blog/immutability-in-csharp-5

I would prefer to see this rather than 'record' which feels kind of alien: name doesn't represent the purpose, syntax is heavily F#-ish etc.

@AlgorithmsAreCool
Copy link

Have you looked at #159 ? It proposes a new immutable type syntax. Your initializer idea might add to that discussion.

@SolalPirelli
Copy link

This can be done with record types (#206) and named arguments.
For instance, if you have a class Point(int X, int Y), you could instantiate it with new Point(X = 0, Y = 42).

@dsaf
Copy link
Author

dsaf commented Feb 4, 2015

@SolalPirelli

Yes, but I don't want a "...type whose semantic meaning is described by the shape of the data..." I just want an immutable class. Record types will be created for a different reason (to facilitate pattern matching).

@dsaf
Copy link
Author

dsaf commented Feb 4, 2015

@AlgorithmsAreCool

Thanks for linking.

@agocke
Copy link
Member

agocke commented Feb 4, 2015

@dsaf

Record types will be created for a different reason (to facilitate pattern matching).

So? If it does what you want, why do you care what it was created for? :)

@dsaf
Copy link
Author

dsaf commented Feb 4, 2015

@agocke

When other people will be reading my code I want them to say "OK, so Student is not going to change it's state after instantiation, so I don't have to worry about that, great." and not "OK, so Student isn't representing a domain object but is rather describing a semantic meaning of some data that can be shaped in different ways... I better check what is that data, what are we matching it for, and where is the actual Student domain object, so that I could rename it to something else to avoid confusion with that pattern matching record type...".

@agocke
Copy link
Member

agocke commented Feb 4, 2015

@dsaf

Don't worry -- no one's going to say that. Welcome to the language design. :)

We discuss the backing state machines of iterators and the semantic structure of records in pattern matching so someone using the code can just say "Oh, that's an IEnumerable+yield" or "Oh, that's an immutable class I can use pattern matching on." Only the designers think about how the sausage is made.

@SolalPirelli
Copy link

@dsaf: Scala has case class, which is close to #206, and case classes are also used as a convenient way to do what you want here, even when no pattern matching is used.

@ashmind
Copy link
Contributor

ashmind commented Feb 4, 2015

Idea based on #159 for the type syntax.

Let's start with:

public immutable class Person {
    public Person(string name) {
        Name = name;
    }

    public string Name { get; }
    public DateTime Birthday { get; }
    ...
}

We want to do var person = new Person("X") { Birthday = birthday }.

What if

public Person(string name) {
    Name = name;
}

actually generated

public Person(string name, [GeneratedForImmutable] Generated_PersonPropertyBag properties)  {
    Name = name;
    // generated
    if (properties.NameSet) Name = properties.Name;
    if (properties.BirthdaySet) Birthday = properties.Birthday;
}

and then var person = new Person("X") { Birthday = birthday } generated

var person = new Person("X", new Generated_PersonPropertyBag {
    BirthdaySet = true,
    Birthday = birthday
})

?

@dsaf
Copy link
Author

dsaf commented Feb 4, 2015

@ashmind #159 is a bit hardcore, frankly I would appreciate even some simple syntax sugar for read-only properties, not a 100% deep immutable types. What if I want this:

public class Sample
{
   public List<string> SomeProperyOne { get; }
   public List<string> SomeProperyTwo { get; }
   public List<string> SomeProperyThree { get; }
   public List<string> SomeProperyFour { get; }
   public List<string> SomeProperyFive { get; }
}

@ashmind
Copy link
Contributor

ashmind commented Feb 4, 2015

@dsaf
But those are lists, you can initialize them in the current C#?
E.g. new Sample { SomePropertyOne = { element1, element2, etc } }

(Given that they are not-null, but they should be!)

@dsaf
Copy link
Author

dsaf commented Feb 5, 2015

@ashmind

The point is that those properties are all mutable themselves, but there is no reason why they cannot be read-only to make the host class "shallow-mutable". If they will be readonly I will not be able to initialize them outside of constructor. #159 would forbid using types such as List<> because they are mutable.

@ashmind
Copy link
Contributor

ashmind commented Feb 5, 2015

@dsaf Sorry, but I don't understand. You can initialize List<T> properties in current C# outside of the constructor, even if the property setter is not available (it compiles to get+Add).

The only problem I see if the class is deep-readonly, but that's similar to #159.

@dsaf
Copy link
Author

dsaf commented Feb 5, 2015

@ashmind List<> was just an example, it could be any mutable type. There is a difference between a read-only and a completely immutable property.

There is no way to do the following in current or proposed C#: have read-only auto-properties that are not necessarily of an immutable type that would be initialized using property initializes. Please refer to my first sample.

@thomaslevesque
Copy link
Member

This feature could easily be done by introducing the notion of a builder class associated to an immutable class.

[BuilderType(typeof(FooBuilder))]
class Foo // immutable
{
    public int Id { get; }
    public string Name { get; }

    public Foo(int id, string name)
    {
        Id = id;
        Name = name;
    }
}

class FooBuilder : IBuilder<Foo>
{
    public int Id { get; set; }
    public string Name { get; set; }
    public Foo Create() => new Foo(Id, Name);
}

With this approach, the compiler would transform this:

var foo = new Foo { Id = 42, Name = "Joe" };

into this:

var foo = new FooBuilder { Id = 42, Name = "Joe" }.Create();

Of course, it requires the developer to write the builder class, but I think this approach is the most flexible. It would also probably be quite easy to implement.

@dsaf
Copy link
Author

dsaf commented Feb 7, 2015

@thomaslevesque the idea is to have less boilerplate code while keeping it readable. I would rather misuse misnamed 'record' classes than have an extra class per every immutable class. Let's move towards Scala not Java. But thank you for being constructive!

@Joe4evr
Copy link

Joe4evr commented Mar 2, 2015

So I came up with a possible solution to this. Should I post it in this thread or make a new proposal?

@gafter
Copy link
Member

gafter commented Mar 2, 2015

@Joe4evr What is your idea?

@agocke
Copy link
Member

agocke commented Mar 2, 2015

@Joe4evr @gafter He also happens to have a marvelous proof that his proposal is optimal, but unfortunately it's too long to fit into a Github comment. ;)

@Joe4evr
Copy link

Joe4evr commented Mar 2, 2015

@agocke lol, I'm not going to Fermat-levels here, but I feel this is much better suited to the language than that inner struct implementation that was suggested 3 weeks ago.

@gafter It comes down to implicitly generating a constructor, just like the parameter-less default constructor that's already made if one isn't provided. This particular one will have all of the getter-only auto-properties as parameters, and when creating an instance with Object Initialization, the compiler looks at what properties are read-only and puts those values into a constructor call. Example:

public class Foo
{
    public string Bar { get; }

    //This constructor would be implicitly generated
    //Properties with regular setters would not have an associated parameter here
    //Parameters could probably be optional, see below
    public Foo(string _Bar)
    {
        this.Bar = _Bar;
    }
}

//Elsewhere....
Foo myFoo = new Foo
{
    Bar = "This is my bar. There are many like it, but this one is mine."
};
//would compile into the following (named parameter used for clarity)
Foo myFoo = new Foo(_Bar: "This is my bar. There are many like it, but this one is mine.");

Since the compiler would know that Bar would correspond to the _Bar parameter in the implicitly generated constructor, it'll move the value into the constructor call in the right place. The rest is just building off of already existing code generation.

Of course, this begs the question: Should this implicit constructor be made in addition to the normal default constructor, or should it be the default constructor for classes with getter-only auto-properties? I'm thinking it could be the latter, if the parameters are made optional and use the default of that parameter's type when not specified (this point is mostly for compatibility reasons).

Thoughts?

@ashmind
Copy link
Contributor

ashmind commented Mar 2, 2015

@Joe4evr seems similar to my struct idea. It is simpler, but in your case any new property will become a binary breaking change -- which I tried to avoid.

@agocke
Copy link
Member

agocke commented Mar 2, 2015

@Joe4evr This looks like the same backing implementation as the pattern matching record class, the difference being that in pattern matching you use the constructor directly rather than setting fields. The record also has much simpler and more succinct syntax IMHO. That is, this is the in-source definition of your example:

record class Foo(string Bar);

And you can declare one simply by new Foo("Bar text blah blah"). Seems like a lot less work to me.

@gafter Please feel free to correct me if I've misunderstood the most recent spec. I don't remember if we finally got rid of that pesky record modifier.

@gafter
Copy link
Member

gafter commented Mar 2, 2015

@Joe4evr @agocke There is no longer any need for the record modifier in the pattern-matching spec.

The intent of the current pattern/records proposal is that the compiler will generate the constructor based on the properties given in the class header. You could put other properties in the body to get additional read-write properties that do not have corresponding constructor parameters.

@Joe4evr If I understand your proposal, the main point is that you could use the object initializer syntax and the compiler will recognize it as constructor parameters instead of separate property assignments. I think that is an attractive idea, and definitely something we're considering.

@thomaslevesque
Copy link
Member

It's an attractive idea, but it comes with a few issues:

  • as mentioned by @ashmind, adding properties breaks binary compatibility
  • when would a constructor be generated? Only when there isn't an explicit one?
  • in which order would the constructor parameters be declared? same order as the properties? In this case, changing the order of the properties would be a breaking change at the source level

@Joe4evr
Copy link

Joe4evr commented Mar 2, 2015

@ashmind That's the one thing, yes, but if the parameters are made optional, then the most one would have to do in case of a library update is rebuild against the new version (provided the library author doesn't make consumers do more, but I think at that point, the author probably would've used an explicit constructor in the first place).

@agocke The main thing is that you can't assign to read-only backing fields outside of a constructor. I don't know how record classes work under the hood, but if you can set their fields directly, they don't sound like read-only.

@gafter That's entirely correct.

@thomaslevesque Yes, not exactly binary compatible, unless the compiler could emit additional metadata.

when would a constructor be generated? Only when there isn't an explicit one?.

That's the idea. If an explicit constructor is provided, then the onus is on the author to make sure all accessible getter-only auto-props can be assigned to.

in which order would the constructor parameters be declared? same order as the properties? In this case, changing the order of the properties would be a breaking change at the source level

Good question, perhaps alphabetical order should then be generated under the hood. The compiler would then place the parameters in their proper location, just like it already does for named parameters.

(5 edits: Markdown, plz)

@agocke
Copy link
Member

agocke commented Mar 2, 2015

@Joe4evr As I said, they generate a constructor, so the constructor parameters become the field values. The only difference between the mapping in your implementation and records is that you've overridden the object initializer syntax to map to a constructor. Records don't contain an extra layer of indirection -- they just use a constructor directly.

@ashmind
Copy link
Contributor

ashmind commented Mar 2, 2015

@gafter

There is no longer any need for the record modifier in the pattern-matching spec.

That's interesting -- which issue has the latest discussion?

@Joe4evr
It is not easy to rebuild a library -- e.g. if you have libA and libB on NuGet using different version of libC -- which is expected to be compatible, but isn't. Of course it happens with other things as well, but the less of those are there, the better.


Overall I don't see a better way than struct if we want to have an object initializer for an otherwise readonly set of properties. However that's only within concepts of current C# -- e.g. record types and non-readonly access by the compiler could be also a solution.

@gafter
Copy link
Member

gafter commented Mar 9, 2015

@ashmind The proposal for records and pattern matching is #206

Also relevant are 'with' expressions as an extension of that... see #206 (comment)

@alrz
Copy link
Member

alrz commented Oct 11, 2015

This is also useful in attribute usages like

public class FooAttribute : Attribute { public string Property { get; } }

[Foo(Property = "value")]

Although, it makes more sense to declare this kind of classes in form of a record type, e.g.

class Student(Guid guid : Guid)
{
    public string Name { get; }
}

There is one problem with this, what if you don't want it to be initializable outside of the enclosing type? Maybe public readonly string Name { get; }, then you can initialize it right away { get; } = ... or in ctor, as you could before.

@stepanbenes
Copy link

@alzr Your proposed initialization syntax would also be a breaking change because assignment operator is expression statement - it returns value that can be then passed to constructor as standard parameter.

@alrz
Copy link
Member

alrz commented Dec 14, 2015

@stepanbense Yes, it actually had missed the point. I just updated my previous comment.

@gafter
Copy link
Member

gafter commented Mar 23, 2017

This has been moved to dotnet/csharplang#322

@gafter gafter closed this as completed Mar 23, 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