-
Notifications
You must be signed in to change notification settings - Fork 1k
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] Required Properties #3630
Comments
To call it out more explicitly: we know we want to do something here, but we need to find a good syntax. I'm looking forward to seeing suggestions from the community as well :). |
My vote is a keyword modifier on public string Foo { get; required set; }
public string Bar { get; required init; } |
My idea there was more public int Prop { get; must init; } The full word was for if it's a modifier on the property itself. |
Obligatory mention of the verbose syntax: public not default int ReadonlyIsALongWordButItIsSomewhatClearWhatItMeans { get; init; } I do not like it, actually, since it tells me something about the property, not that I should initialize it. My vote is with Here's some more ideas: public int Prop { get; do init!!; }
public int Prop { get; explicit init; }
public int Prop { get; init!!; }
public int Prop { get; init else throw; } |
Got it, so it would be: public mustinit string Foo { get; init; } You could argue that the modifier on the property makes it easier to see which properties are required vs. having to potentially scan for the accessors. I still think I'd lean towards |
What's the reasoning being |
Updated the proposal with a new alternative section on "Initialization Debt", which builds on this proposal as a general way of communicating what properties must be initialized in a more flexible fashion. |
@Richiban consider the first example: class Person
{
string FirstName { get; init; }
string MiddleName { get; init; }
string LastName { get; init; }
} Not everyone has a |
I get it, but why not simply let them set null explicitly?
It just seems like a significant weakening of a feature (or, perhaps, a requirement to develop two features) to make the case of optional properties a little easier. EDIT: Even better: use the concept of property initial values to indicate optionality: class Person
{
string FirstName { get; init; }
string MiddleName { get; init; } = null; // This is optional because it's been given a default value
string LastName { get; init; }
} |
Orthogonal concerns. |
Are required and initializable properties being considered as orthogonal concepts? Or is required an extra that may only be applied to initializable properties? I'd like to be able to write property declarations that trigger a compilation failure if I fail to initialize the properties in my own constructors: public class Person
{
public string KnownAs { get; private required set; }
public string FamilyName { get; private required set; }
public string LegalName { get; private required set; }
public Person(string knownAs, string familyName)
{
KnownAs = knownAs;
FamilyName = familyName;
// Compiler error here because LegalName has not been initialized
}
} I'd find this useful for expressing intent, especially on classes that I expect will need to be modified in the future as the system develops over time. For example, if I added public string Honorific { get; private required set; } to the above class, having the compiler ensure that I correctly initialized the property in every constructor (including the one in a different file, thanks to a partial class declaration) would be really useful. |
Interesting scenario. I'm under the impression that this proposal covers the metadata and enforcement of consumers of the class, not of the class itself. |
It is an interesting scenario, and one I think could be covered by the initialization debt extension. In that world, those 3 things would be part of the debt, but because they're private to your class you wouldn't be able to publish them as part of your debt contract. They would have to be assigned in the constructor. |
I like
|
Sometimes it's good to pause and think through what someone says before instantly rejecting what they are saying. When we do so, we see that the true orthogonal concerns here are I suspect I'm not alone in being completely perplexed by the opening statement "The number 1 misconception about Therefore, let's start with the requirement that it is mandatory. That's what most people will expect it to be and that's what most people will want it to be most of the time. So how do we address @333fred's conundrum? class Person
{
string FirstName { get; init; }
string MiddleName { get; init; } // many people don't have a first name so it shouldn't be mandatory
string LastName { get; init; }
} Well first of all, if it's not mandatory, it has no right being declared as class Person
{
string FirstName { get; init; }
string? MiddleName { get; init; } = null;
string LastName { get; init; }
} Now, This feature is a nice one and it would be good to add it to For a mutable class Person
{
string FirstName { get; init set; }
string? MiddleName { get; set; }
string LastName { get; init set; }
} Now, This also then solves @theunrepentantgeek's scenario: public class Person
{
public string KnownAs { get; private init set; }
public string FamilyName { get; private init set; }
public string LegalName { get; private init set; }
public Person(string knownAs, string familyName)
{
KnownAs = knownAs;
FamilyName = familyName;
// Compiler error here because LegalName has not been initialized
}
}
|
So then how do you propose to make a
|
As explained above, by using
As clearly stated by the OP, many (likely most) people assume and will continue to assume it does mean mandatory because it makes sense for it to be mandatory for most cases. So I've turned @333fred's proposal on its head with my counter proposal which allows it to be mandatory and meets all the others requirements mentioned in this thread all without adding an extra keyword that has to be used in most scenarios and is only omitted in exceptional cases. |
I admit in my first post that I never considered the case of required but mutable properties, but I would still argue for the language design philosophy of:
in the name of safety and having a language that will cause developers to fall into the pit of success. |
Nothing about |
A few points:
|
@DavidArno but that's not what it is. Unfortunately, properties from C# 1.0 were optional by default, and so that's the world we're going to have to live with. In fact, other than conflating
Nullable was not enabled in that code snippet, it cannot be Your proposal paints a nice picture, one I even like. However, it would require changes to large swaths of code, and if we were going to do that I don't think it goes far enough. I'd much rather extend the concept of definite assignment to all properties of an object, and have constructors/factory methods export explicit contracts of "these things must be initialized before this object is considered valid". Then we could have a world like this: #nullable enable
// Maybe the contract on a default constructor could be implicit, but I'm just putting it on the
// class declaration for example's sake
[RequiresInit(nameof(Person.FirstName), nameof(Person.LastName)]
public class Person
{
string FirstName { get; init; }
string? MiddleName { get; init; } = null;
string LastName { get; set; }
}
public static class PersonFactory
{
[RequiresInit(nameof(Person.LastName)]
public static Person MakeMads() => new Person { FirstName = "Mads" };
}
var kristensen = PersonFactory.MakeMads() { LastName = "Kristensen" };
var torgersen = PersonFactory.MakeMads() { LastName = "Torgersen" };
var error = PersonFactory.MakeMads(); // Error: "LastName" is uninitialized" In this world of mutable last names, no extra keyword would be required here: you didn't initialize the property, so it's part of the "debt" that is exposed from a constructor or factory method. But as much as I like this system, it presumes that properties are required by default, which is unfortunately just not true today, and not likely something that we will be able to change.
What we do in the name of backcompat, @Richiban. What we do in the name of backcompat. |
The concept of initialization debt could certainly be extended to fields as well.
That's what the more general initialization debt extension would cover.
Until we get non-defaultable structs I think this is a pretty moot point. It's something we certainly want, but without the feature it doesn't matter. |
I still don't understand how Please tell me how can something been ONLY setable during initialization, but then not be mandetory. That property is not setable after, so what is the value going to be? Is it default? Is it null? In this case I agree with what has been set, the property/field needs to be either nullable or need to be preinitialized (not sure if this makes even sense for something that is called init). Are we really talking about introducing a keyword or attribute just so the developer doesn't have to initialize during initialization? I rather write var person = new Person
{
Firstname = "John",
Middlename = default,
Lastname = "Doe"
} over introducing a new keyword or Attribute. Still don't understand what the state of properties will be if they are not initialized even though they can only be set during initialization 🤔 Edit: autocorrection |
It's going to be whatever default was provided, or
Don't forget that initialization includes field/property initializers (including property |
Do I understand correctly that C# 9 As for this issue, if both Do we expect this to be a commonly used feature? Because an attribute on the property might do the job as well... |
|
See: #6780 |
I think you have an impedance mismatch. You’re trying to put required-ness into a positional record. But for a positional record, positional parameters are constructor parameters – required by default. To get record semantics with property initializers, elide the primary constructor. (But since you’re using a The short answer is that your parameters seem non-required because you’re using a value type. Consider using a reference type – |
@KennethHoff I think the underlying issue here is that all |
@HaloFour I see...
I don't like constructors as it's difficult to know what I'm actually assigning - is the 3rd parameter email or password? etc.. (Primitive Obsession notwithstanding..) @TahirAhmadov I knew of that actually, but didn't consider in record-struct land |
Are we now back to one of the initial complaints regarding NRT and init properties? My take on this was always, and still is, the compiler should not analyze init properties before the init phase is completed.
|
@thomaseyde can you edit your post? It's full of email goop. I'd do it, but I'm genuinely not sure what parts to keep. |
I know I'm coming in late to this, but do we need to consider types with multiple constructors? public class Person
{
public String FirstName { get; }
public String MiddleName { get; }
public String LastName { get; }
public Int32 Age { get; set; }
public Person(String firstName, String lastName)
{
FirstName = firstName;
LastName = lastName;
}
public Person(String firstName, String middleName, String lastName)
{
FirstName = firstName;
MiddleName = middleName;
LastName = lastName;
}
} This clearly states that we either need to provide 2 or 3 properties when initializing (via constructor). We can then already use very similar initialization code to access these constructors like so: var person = new Person(
firstName: "hello",
lastName: "world")
{
Age = 21
}; It feels like we're trying to make this slightly neater at the end of the day - so something like this might work: var person = new Person
{
@firstName = "hello",
@lastName = "world",
Age = 21
}; // Maps explicitly to the (String firstName, String lastName) constructor I used This allows us the flexibility of explicitly defining the allowed combinations of required/optional parameters through the definition of constructors, as we do already. Personally, I like the idea of a |
Good point. Although I think your example shows that the MiddleName is in fact not required, hence there is no need for the required keyword. If the goal is to make it immutable and avoid constructors, the best option is to make MiddleName init-only. |
DISCLAIMER: Example for demonstration purposes only 😉 Was the simplest example I could come up with that demonstrated the syntax. |
I don't think anyone mentioned this before, apologies if I missed it in the many comments that are in this issue. I encountered an unfortunate consequences of the new For example, linq2db supports writing the SET part of an UPDATE sql statement like so: class Product
{
public required int Id;
public required string Name;
}
db.Products
.Where(p => p.Id == 3)
.Update(_ => new() { Name = "John" }); // CS9035
// UPDATE Product SET Name = 'John' WHERE id = 3 This is not the only syntax linq2db provides, but it's a very friendly and convenient one when you want to set many different properties at once. Of course, the problem is that C# complains that the initializer doesn't set I wonder if CS9035 should be muted inside Expressions. This is just one example, but one can do lots of creative things with Expressions, e.g. initializing some members magically. I can't remember exactly what, but I recall there was another C# error/feature that is disabled inside Expressions, for similar reasons? PS: it would be incredibly nice if the new |
@jods4 This exact problem was recently mentioned in the Entity Framework Core repository, too. There was a dedicated topic for it at dotnet/efcore#28557 Other relevant comments:
AFAIK, this issue was never solved, so I've posted a comment nagging @roji about it 🙂 (dotnet/efcore#795 (comment)) |
I've always thought it was weird that EF used |
@Bosch-Eli-Black @HaloFour EF Core doesn't use |
@Bosch-Eli-Black re EF Core:
Sure it was - we simply decided not to use More on this in dotnet/efcore#795 (comment). |
I hope my comment doesn't get discarded because EF Core opted for a different solution.
|
The current implementation (.NET 7) has a problem and disappoints me a lot; it doesn't allow such a code like: // Error in this line
Console.WriteLine(new Foo().Bar);
public class Foo {
public required string Bar { get; init; }
Foo() {
// I want to make an assignment only when the caller is not a form like `new Foo { Bar = "some string" }` somehow
Bar ??= "Hello world!";
}
} I want constructors to be able to initialize properties with both I just wish all nulls in properties and fields of non-nullable types would be wiped out of all maintained .NET projects all over the world. |
That's just never going to happen. It would break almost every C# program in existence, prior to NRT's. |
@tats-u I think what you want is a property which has a default value, but can be set in init block to set a custom value. In this case, you don't really need Also, I think you expect the ctor to be executed after the init block, which is not the case - the init block is executed after the ctor. If you want to make changes after the init block is executed, there is #6591, which proposes "final initializers". |
@KieranDevvs That's why I chose "wish" instead of "hope". It seems much more difficult than I thought though. Foo(/* some arguments */) {
// or an alternative expression to detect if the property `Bar` has not been set yet
if (Bar == null)
{
// complex processing according to the arguments
Bar = /* complex expression according to the arguments */;
}
} |
@tats-u but you don't need |
@TahirAhmadov This will be a non- |
var $temp = new Foo();
$temp.Bar = "Baz";
var foo = $temp; The constructor is always invoked first, property initializers always come second. |
@HaloFour Do you mean there's no way for constructors to tell whether the initial property values has been passed in the initializers, right? private string bar;
public string Bar { get { return bar; } init { bar = value; } }
Foo (/* args */) {
bar = /* computed values */;
} |
Correct. Initializers are mostly just syntax candy for assigning values to the property setters immediately after the constructor is called. With |
One reason I would like to vote for method calls in initializers is this-sometimes we want to do some post-initialization work after properties are set. For example: var foo = new Foo()
{
Bar = "Baz",
Init(),
}; We can have fields/properties and recently events initialization in initializer. For completeness, we should support methods, though I can imagine that there might be some design work related to the |
@HaloFour I found fields & constructors and properties and initializers are independent when |
It would be cool if something like this:
Now we have 2 ways of initializing things and which way to choose???
Why can't 1 work also like 2? I prefer how clean 1st way is and I also prefer to initialize longer things without constructor. It's a bit dumb that these things don't work interchangeably, the compiler could at least allow using object initializer for stuff in constructors and compile that just like it was passed to a constructor. Someone should make D# lol... |
Required Properties
Specification: https://github.com/dotnet/csharplang/blob/master/proposals/csharp-11.0/required-members.md
Design meetings
https://github.com/dotnet/csharplang/blob/master/meetings/2020/LDM-2020-09-16.md#required-properties
https://github.com/dotnet/csharplang/blob/master/meetings/2020/LDM-2020-12-07.md
https://github.com/dotnet/csharplang/blob/master/meetings/2021/LDM-2021-01-11.md
https://github.com/dotnet/csharplang/blob/master/meetings/2021/LDM-2021-03-03.md#required-members
https://github.com/dotnet/csharplang/blob/main/meetings/2021/LDM-2021-10-25.md#required-members
https://github.com/dotnet/csharplang/blob/main/meetings/2021/LDM-2021-12-15.md#required-parsing
https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-03-21.md#open-question-in-required-members
https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-03-23.md#open-questions-in-required-members
https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-05-02.md#effect-of-setsrequiredmembers-on-nullable-analysis
https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-11-30.md#revise-membernotnull-for-required
The text was updated successfully, but these errors were encountered: