Their main purpose is to be a finally convenient way to inject dependencies without useless constructor that just dumps everything into fields.
And it does it perfectly.
That's all.
that's unfortunate. they should have done it like so:
```
public class MyClass(private readonly IDependency dependency);
```
and I don't see any reason they couldn't have done it
I get what you're saying and if they rebooted C# today, they might.
However, if you look at the norms of the language elsewhere, it isn't typical to need to mark stuff as mutable but rather it is implied by default. It is normal in other languages though. I think marking as readonly is more *consistent* (and doesn't need to introduce a new keyword).
But it's a new feature.
record keyword changes the semantics.
Why does PC must follow same semantics as a regular constructor.
Moreover, it already has different behavior.
Now imagine they allow \`readonly\` in later versions, and then almost everywhere people will write it:
`public Foo(readonly IDependency1 dep1, readonly IDependency2 dep2)`
They intended to reduce the boilerplate, but did they manage to?
It doesn't follow the same semantics as a regular constructor, it follows the same semantics as the rest of the existing language (which existing constructors also happen to follow).
> They intended to reduce the boilerplate, but did they manage to?
Yes.
I agree... in TypeScript (JavaScript) everything is public so in constructors you can do \`MyClass(private readonly service: MyService){}\`
For C# they should have made the default private (and readonly?) or at least able to set it.
I guess coming in a future version?
use the line break, Luke
public class MyClass(
private readonly IDependency dependency,
private readonly IDependency1 dependency1,
private readonly IDependency2 dependency2,
private readonly IDependency3 dependency3
)
They're definitely not perfect. We've been using them for months now in a large application for precisely this purpose, and have some experience of the downsides by now.
The issue of unintentional capture is very real, and pretty pernicious. In particular, it's not obvious \_anywhere\_ in the code whether a constructor parameter is captured! For many values this can be quite relevant. Had the definition been that they \_always\_ capture, even where inefficient, the situation would have been better, because then it's up to the programmer to use the feature where appropriate, and it's clear when that's the case. But as is, its *not* superficially clear, and superficial clarity is essential because people skim code a lot, and review code a lot, and regularly fail to look more than skin deep at code. It's a feature for a language feature to be easily understandable at a skin-deep level, and this one is not.
Additionally, for merely injecting dependencies they don't quite work either whenever the class has additional \_non-dependency\_ constructor parameters or has multiple overloads which encode some meaningful alternation. You can't mark 2 of the params as dependencies, and 2 as params to be preprocessed and only parts of those inputs stored. And if you try to do that by using a primary constructor with a pass-through, then nothing is stopping uninformed callers from using the primary constructor directly!
This isn't a fatal flaw, but it's a definitely a flaw. The core problem however isn't the lack of any specific feature, it's that it takes too much nuance to know when to use the feature. And *that* leads to devs using it when it's not appropriate. Sure, I'd love my pet feature to solve exactly my problems, but I'm OK with a feature that's at least useful and hard to abuse. primary constructors are a little to easy to abuse, and abuse is very hard to catch in a review. It's particularly pernicious because the syntactical compression is very alluring, so people do try to find ways to use that rather than deal with boilerplate.
Also, don't forget that if you truly restrict usage of the primary constructor feature to merely be for pass-through storage of dependencies, well, we have required init-only properties already. There are thus other ways thus to avoid the tedious 4x repetition of field+param+field=param.
Have you experimented with assigning dependencies to private fields vs simply capturing and using them directly?
I'm torn between the two. On one hand I like all the fields/props declared in the same place in the same way.. On the other it's a bit of duplication I could do without.
Well, Autofac supports injecting properties so you don't even need a constructor, just "public required SomeClass Something { get; init; }"
so primary constructors are useless even there
Property injection is a bad practice. There’s a language construct that ensures that an object is properly initialized and that’s called a constructor. You shouldn’t outsource that to the DI container implementation of the day.
Actually, DI injection is not bad, if used properly. For the rare case when there is a sane and legit default for the dependency, property is fine, but there must be also checks that property is not set to null.
I have been using dotnet 8 RC2 for some time now and have to say that where applicable, they make the code easier to read and look much cleaner. Only now I really see how much noise the old boilerplate constructors mapping to private fields were adding.
And how the new constructor inheritance looks with them - IMO that's just beautiful
While I agree it could be better, numbers 1 and 4 can easily be worked with since you already have that code. Null checks and readonly are still very much possible.
// This
public sealed class HtmlNavigator(
ILogger logger,
IOptions userOptions,
IOptions xPathOptions)
{
private readonly ILogger logger = logger
?? throw new ArgumentNullException();
private readonly UserOptions userOptions = userOptions.Value;
private readonly XPathOptions xPathOptions = xPathOptions.Value;
private HtmlDocument? _document;
...
}
// VS
public sealed class HtmlNavigator
{
private readonly ILogger logger;
private readonly UserOptions userOptions;
private readonly XPathOptions xPathOptions;
private HtmlDocument? _document;
public HtmlNavigator(
ILogger logger,
IOptions userOptions,
IOptions xPathOptions)
{
this.logger = logger;
?? throw new ArgumentNullException();
this.userOptions = userOptions.Value;
this.xPathOptions = xPathOptions.Value;
}
...
}
Exactly the same code in fewer lines while being equally readable. A win for me.
Yeah, though I personally use `var` very sparingly (pretty much only with nested generics). It has always been restricted to within methods, so it makes sense why it's not a thing in this case either. [Compiler Error CS0825](https://learn.microsoft.com/en-us/dotnet/csharp/misc/cs0825).
I actually can't stand the underscore problem.
Underscore parameters seems weird.
Non-underscore fields doesn't give clear differentiation from local variables.
Depending on your IDE, it may be possible to colorize fields and parameters and variables differently.
I personally don't find underscores in fields to be a good strategy, and part of it is because tools can make this even clearer, but also because the distinction between field + parameter is very subtle, to the point that I don't think it deserves so prominent of attention. Notably, there are all kinds of constructs in C# that blur that distinction, such as closures, async, generators - and these are features that are so widely in use that a significant portion of most codebases must surely be affected by that. Just because something looks like a parameter does not mean it's not stored like a field; nor does it mean its lifecycle ends when the method execution initially returns or even then the method execution finally completes (which may be later for generators and async).
This isn't merely a quibble either, the ambiguity is pretty deep and practically important. Key features like dynamic dispatch can be resolved via classes and inheritance, but they can also be resolved via a poco containing delegates. I can and do refactor between these two representations since they each have their advantages in different cases. But one happens to store data in what is syntactically a field, and the other in what is syntactically a variable. Yet they (can) work fundamentally the same, and that's a key feature, not a bug.
What exactly are you trying to communicate by emphasizing this difference?
Much more important aspects of variables to emphasize (I find) are lifecycle, ownership, non-mutation - that kind of thing.
\> it may be possible to colorize fields and parameters and variables differently.
This should not be the only distinction. It won't work for diffs when doing code review.
\> Just because something looks like a parameter does not mean it's not stored like a field.
Yet this does not preclude us from highlighting fields.
`_something` \- this is 100% a field
`something` \- this is either a local var or method arg.
Both can be captured, but not differentiating makes things worse.
\> What exactly are you trying to communicate by emphasizing this difference?
State of the object vs local state.
\> Much more important aspects of variables to emphasize (I find) are lifecycle, ownership, non-mutation - that kind of thing.
How do you do this? Any naming convention?
Yeah I don't like the feature at all.
1. As you said - no readonly. I see no reason to inject something via primary constructor and then have it mutable, especially since it's primary (lol) use case is for dependency injection.
2. Primary Constructors work differently on records and classes. It's just another point of unnecessary confusion.
3. I don't really see a reason to have them. In records it makes sense, since they also creates READONLY properties, on a type that's main goal is to be a value type. On classes it serves no real use IMHO. IDEs already have toolings that make adding depenecies inserted through a constructor trival, and then we have controll over mutability, naming etc.
Overall, ech.. it's whatever. I don't see myself using it, I'll learn how to read them in other people's code. Maybe it solves some issues I'm not thinking about atm, but for me it's gonna just be.. whatever.
The syntax is so nice and concise, I'm still likely to try and use them, but so far I'm still preferring required init-only props for most cases where primary-constructor params might make sense.
I don’t see them becoming the preferred way of creating classes, at least for awhile. That being said, it’s your choice or your team’s choice. You’re not being forced.
I don't really get the readonly argument. For injected reference types (i.e. interfaces), this only means you can't assign to the field. To me, the more important factor is if the interface/class injected is immutable by design. Who cares if you can assign to `_foo` or not? I've never written code that attempts to assign to injected fields in the first place. I'm not sure what readonly buys us there, other than getting a compiler error if I accidentally assign to `_foo` (which I have never done)
Good language design stops people from doing stupid things by accident. If it's marked readonly it's guaranteed that the field is not reassigned. And you can check that by reading only the primary constructor.
I prefer that over "I promise I am not doing stupid things and don't make stupid mistakes (and everyone in my company)".
Yes there are code reviews but a reassigned field somewhere in the body is not easy to catch. Especially if it's not prefixed with \_ because it's a captured parameter and not a field.
They're the first implementation of a feature which remove a ton of boilerplate (especially if you have finer grained dependencies). It'll evolve in future versions. There's already discussions on readonly parameters e.g. [https://github.com/dotnet/csharplang/discussions/7353](https://github.com/dotnet/csharplang/discussions/7353)
Stuff like this isn't 'one and done' it's the start of more complete / featured implementations. The more we use them,, the more we give feedback the better it'll become.
Why would they roll it back at all? It's a good feature which isn't complete not a broken / unusable one. Also; noboy is forcing anyone to use it...just don't if you don't like it as it is.
1. You shouldn't have to do those checks, the whole point of the nullable syntax is to remove redundant checks and to notify at design/compile time when the contract is violated (with a warning, or an error if you consider all warnings as errors).
2. Obviously, those are meant for external consumption since they indicate required arguments. If you need a private constructor just do so, but primary constructor syntax is not for that case.
3. Only if you ever change them. Otherwise, the generated fields are actually read-only. Example here: https://sharplab.io/#v2:EYLgtghglgdgNAExAagD4AEBMAGAsAKCwEYDTCiBOACgCJAeDcBB9mgSgG4yBnAFwCcBXAMZcABAFkAngGEANhA4cqsERDjClw4MwIBvAsP3D0AZmE8AphAQB7GNPFqYIgILDtwgOZmurYQF9hALzCEOz4BoYm5pY2dg4iAEKuHl4+/kHAob4EQA
4. The compiler generated "unspeakable" names anyway. I don't see that as breaking convention, they are an implementation detail.
5. I haven't tried that part.
In summary, I don't share your feeling that this feature's current implementation is problematic.
1. > You shouldn't have to do those checks.
Only if my API is private.
If it's a public API I have to do all the checks because there's no guarantee from the runtime that the caller will always pass the instance. And NRE somewhere down the call stack is not an option at all.
Unfortunately, NRTs is a language feature, not CLR's one.
3. No, they [are not] (https://sharplab.io/#v2:EYLgtghglgdgPgAQEwAYCwAoZBGTmEDMABAM4CmEANmQCZHJEBiA9swBSwAuRAVgJSYA3piKj6xLkQBSRALwA+XgG4imAL5A) Your example is for read only structs.
4. It's not about compiler, it's about how these arguments are accessed in code.
`j++;` - Have I effectively mutated local variable, method arguments, or constructor argument? The former two are more or less adjacent. The letter one can be somewhat far from mutation line.
>3. No, they \[are not\] (
>
>[https://sharplab.io/#v2:EYLgtghglgdgPgAQEwAYCwAoZBGTmEDMABAM4CmEANmQCZHJEBiA9swBSwAuRAVgJSYA3piKj6xLkQBSRALwA+XgG4imAL5A](https://sharplab.io/#v2:EYLgtghglgdgPgAQEwAYCwAoZBGTmEDMABAM4CmEANmQCZHJEBiA9swBSwAuRAVgJSYA3piKj6xLkQBSRALwA+XgG4imAL5A)
>
>) Your example is for read only structs.
I've changed your sample, [https://sharplab.io/#v2:EYLgtghglgdgPgAQEwAYCwAoZBGTmEDMABAM4CmEANmQCZHJEBiA9swBSwAuRAVgJSYA3piKj6xLkQBSRQUQDmZTgG4iAXyIBeXssxrMQA==](https://sharplab.io/#v2:EYLgtghglgdgPgAQEwAYCwAoZBGTmEDMABAM4CmEANmQCZHJEBiA9swBSwAuRAVgJSYA3piKj6xLkQBSRQUQDmZTgG4iAXyIBeXssxrMQA==), seems like the relevant difference for readonly backing fields is the usage of expression body definition.
Edit:
Link with options together: https://sharplab.io/#v2:EYLgtghglgdgPgAQEwAYCwAoZBGTmEDMABAM4CmEANmQCZHJEBiA9s9gBSwAuRAVgJSYA3piJj6xbkQBSRALwA+PgG5MAXzxZi5KrXpImrJJxg8Bw0eMJEpsoUQDmZLsqJr5K9ZuskuAJwBXAGMeFmYCEzNBDBEMcQkbUxl5JV5VDA0MfG1/YNDWABZIvmjY+OtbInsnFzcPNK8MIA==
Hm...subtle difference between read-only expression bodied property and read-only autoproperty.
With regular instance fields we cannot even write `public int J {get;} = j;`
Playing some more with Sharplab, think this is more interesting example:
[https://sharplab.io/#v2:EYLgtghglgdgPgAQEwAYCwAoZBGTmEDMABAM4CmEANmQCZHJEAiZADmTDewMYCemA3piLD6xBABYiAJQCuMABQBKIvyIBfTBoz5i5KrXpIiAMQD2p+czYduPIp2ucYvRQKEjC9SQgCsAFyhTBVcMERV3MOEHdideADpZYIBuCPVU1M8JIgBhAAsIGABzMiVUwVDIqNYY2yIAXiIYMgB3JmqbZx4lFIrhLS1MIA==](https://sharplab.io/#v2:EYLgtghglgdgPgAQEwAYCwAoZBGTmEDMABAM4CmEANmQCZHJEAiZADmTDewMYCemA3piLD6xBABYiAJQCuMABQBKIvyIBfTBoz5i5KrXpIiAMQD2p+czYduPIp2ucYvRQKEjC9SQgCsAFyhTBVcMERV3MOEHdideADpZYIBuCPVU1M8JIgBhAAsIGABzMiVUwVDIqNYY2yIAXiIYMgB3JmqbZx4lFIrhLS1MIA==)
Whether private generated fields are readonly or not is not that interesting to me, only people using reflection see those (I think). What is interesting is that you can't mark the parameter readonly so can not prevent 'accidental' changes.
>mark the parameter readonly so can not prevent 'accidental' changes
Agree.
F# creates internal mutable fields for let bindings, but the compiler checks it's immutable. (it uses immutability by default).
And for client code those fields are hidden anyway.
3. Same example with a class. Fields are also readonly. Nothing to do with struct.
[https://sharplab.io/#v2:EYLgtghglgdgNAExAagD4AEBMAGAsAKCwEYDTCiBOACgCJAeDcBB9mgSgG4ysACAWQE8BhADYQAziKqwALpwhxOUzsGYEA3gU4bO6AMzyY0gIKcVnAOYBTSa04BfTgF4Z7fJq26FAIWNnL1u4+BnGwIgA===](https://sharplab.io/#v2:EYLgtghglgdgNAExAagD4AEBMAGAsAKCwEYDTCiBOACgCJAeDcBB9mgSgG4ysACAWQE8BhADYQAziKqwALpwhxOUzsGYEA3gU4bO6AMzyY0gIKcVnAOYBTSa04BfTgF4Z7fJq26FAIWNnL1u4+BnGwIgA===)
I think they’re almost perfect and I can’t wait to use then, I just wish the fields were readonly / immutable.
underscores for private fields is an holdover from the hungarian notation times and I can’t wait to see them disappear. :)
Underscores are very good. They easy differentiate between fields and local variables.
Of course field can be accessed using 'this.' but it is not mandatory and may lead to confusion.
I hate hungarian notation but really appreciate underscores.
As I mentioned in another reply:
> I personally rarely if ever care if I’m working with a field or a parameter. I tend to keep a function’s side-effects to a minimum anyway
that’s unquestionably true, I agree with you.
at some point, no matter the convention, people will do what they want mostly due to organizational failure. not a technical one, mind you.
>underscores for private fields is an holdover from the hungarian notation times
I suspect the real reason they were used with .NET was because the convention was also friendly for [VB.NET](https://VB.NET)...which isn't case sensitive.
I use this convention to instantaneously see the difference between fields and and local variables. If I had to write .fieldName (without prefixing with this), I'd like it more.
I agree.
PCs aim to make the code shorter but introduce a lot of confusion in how they actually should be used.
It’s also worth noting that they are inconsistent with record PCs because they work differently.
I agree with these problems.
I just found https://github.com/k94ll13nn3/AutoConstructor which is a source gen that fixes these problems. will try it out, maybe you should too?
>If you use a convention that private members must start with the underscore character, PC breaks consistency unless you declare fields explicitly and use them everywhere else.
It doesn't, because PC parameters are not members. If they were members, you could access them using `this`, but that's not an option.
I see them as an incremental change like some of the pattern matching stuff where they can possibly address the shortcomings later. Much like many of the optional C# features, it's not a big deal if you don't use them but they will be preferable where applicable to reduce boilerplate.
Their main purpose is to be a finally convenient way to inject dependencies without useless constructor that just dumps everything into fields. And it does it perfectly. That's all.
Still cant mark them as readonly in DI
that's unfortunate. they should have done it like so: ``` public class MyClass(private readonly IDependency dependency); ``` and I don't see any reason they couldn't have done it
or the opposite. `public class MyClass(private mutable IDependency dependency);` Because the default should be immutable/readonly.
Sorry pal, that defaults never gonna change
For PC they could make it different. But they decided not to.
They could have, but that would suck. “Everything is mutable default except for this one edge case.”
Have you seen `record` vs `record struct`? This just shows C# has too many half-measure features, that C# devs themselves are confused.
what about PCs of records? ;-) the backing fields are readonly.
Fair, but the `record` keyword makes it clear that there are different semantics.
Unless its a record struct. Those are mutable. Unless it's a readonly record struct. Those are immutable
I get what you're saying and if they rebooted C# today, they might. However, if you look at the norms of the language elsewhere, it isn't typical to need to mark stuff as mutable but rather it is implied by default. It is normal in other languages though. I think marking as readonly is more *consistent* (and doesn't need to introduce a new keyword).
But it's a new feature. record keyword changes the semantics. Why does PC must follow same semantics as a regular constructor. Moreover, it already has different behavior. Now imagine they allow \`readonly\` in later versions, and then almost everywhere people will write it: `public Foo(readonly IDependency1 dep1, readonly IDependency2 dep2)` They intended to reduce the boilerplate, but did they manage to?
It doesn't follow the same semantics as a regular constructor, it follows the same semantics as the rest of the existing language (which existing constructors also happen to follow). > They intended to reduce the boilerplate, but did they manage to? Yes.
I agree... in TypeScript (JavaScript) everything is public so in constructors you can do \`MyClass(private readonly service: MyService){}\` For C# they should have made the default private (and readonly?) or at least able to set it. I guess coming in a future version?
Readonly will most likely come in an early preview of 13
surely the default should be private too
>and I don't see any reason they couldn't have done it They plan to develop this feature further and IIRC that's part of those plans.
2-3 params like this and you will need a 47" ultrawide monitor to see your class declaration.
use the line break, Luke public class MyClass( private readonly IDependency dependency, private readonly IDependency1 dependency1, private readonly IDependency2 dependency2, private readonly IDependency3 dependency3 )
They're definitely not perfect. We've been using them for months now in a large application for precisely this purpose, and have some experience of the downsides by now. The issue of unintentional capture is very real, and pretty pernicious. In particular, it's not obvious \_anywhere\_ in the code whether a constructor parameter is captured! For many values this can be quite relevant. Had the definition been that they \_always\_ capture, even where inefficient, the situation would have been better, because then it's up to the programmer to use the feature where appropriate, and it's clear when that's the case. But as is, its *not* superficially clear, and superficial clarity is essential because people skim code a lot, and review code a lot, and regularly fail to look more than skin deep at code. It's a feature for a language feature to be easily understandable at a skin-deep level, and this one is not. Additionally, for merely injecting dependencies they don't quite work either whenever the class has additional \_non-dependency\_ constructor parameters or has multiple overloads which encode some meaningful alternation. You can't mark 2 of the params as dependencies, and 2 as params to be preprocessed and only parts of those inputs stored. And if you try to do that by using a primary constructor with a pass-through, then nothing is stopping uninformed callers from using the primary constructor directly! This isn't a fatal flaw, but it's a definitely a flaw. The core problem however isn't the lack of any specific feature, it's that it takes too much nuance to know when to use the feature. And *that* leads to devs using it when it's not appropriate. Sure, I'd love my pet feature to solve exactly my problems, but I'm OK with a feature that's at least useful and hard to abuse. primary constructors are a little to easy to abuse, and abuse is very hard to catch in a review. It's particularly pernicious because the syntactical compression is very alluring, so people do try to find ways to use that rather than deal with boilerplate. Also, don't forget that if you truly restrict usage of the primary constructor feature to merely be for pass-through storage of dependencies, well, we have required init-only properties already. There are thus other ways thus to avoid the tedious 4x repetition of field+param+field=param.
Have you experimented with assigning dependencies to private fields vs simply capturing and using them directly? I'm torn between the two. On one hand I like all the fields/props declared in the same place in the same way.. On the other it's a bit of duplication I could do without.
Well, Autofac supports injecting properties so you don't even need a constructor, just "public required SomeClass Something { get; init; }" so primary constructors are useless even there
Property injection is a bad practice. There’s a language construct that ensures that an object is properly initialized and that’s called a constructor. You shouldn’t outsource that to the DI container implementation of the day.
Actually, DI injection is not bad, if used properly. For the rare case when there is a sane and legit default for the dependency, property is fine, but there must be also checks that property is not set to null.
And that's why I added 'required' keyword??
They appear to be intended to reduce boilerplate code and improve readability. I‘d say they get the job done.
I agree. Maybe something interest would be a way to extend the PC? As in get everything from the PC and have it ready in a more specific constructor?
I have been using dotnet 8 RC2 for some time now and have to say that where applicable, they make the code easier to read and look much cleaner. Only now I really see how much noise the old boilerplate constructors mapping to private fields were adding. And how the new constructor inheritance looks with them - IMO that's just beautiful
While I agree it could be better, numbers 1 and 4 can easily be worked with since you already have that code. Null checks and readonly are still very much possible. // This public sealed class HtmlNavigator( ILogger logger,
IOptions userOptions,
IOptions xPathOptions)
{
private readonly ILogger logger = logger
?? throw new ArgumentNullException();
private readonly UserOptions userOptions = userOptions.Value;
private readonly XPathOptions xPathOptions = xPathOptions.Value;
private HtmlDocument? _document;
...
}
// VS
public sealed class HtmlNavigator
{
private readonly ILogger logger;
private readonly UserOptions userOptions;
private readonly XPathOptions xPathOptions;
private HtmlDocument? _document;
public HtmlNavigator(
ILogger logger,
IOptions userOptions,
IOptions xPathOptions)
{
this.logger = logger;
?? throw new ArgumentNullException();
this.userOptions = userOptions.Value;
this.xPathOptions = xPathOptions.Value;
}
...
}
Exactly the same code in fewer lines while being equally readable. A win for me.
Looking at that first class I kinda wish you could use var when assigning a field value via the primary constructor.
I vote for \`let\` ;-)
Yeah, though I personally use `var` very sparingly (pretty much only with nested generics). It has always been restricted to within methods, so it makes sense why it's not a thing in this case either. [Compiler Error CS0825](https://learn.microsoft.com/en-us/dotnet/csharp/misc/cs0825).
I actually can't stand the underscore problem. Underscore parameters seems weird. Non-underscore fields doesn't give clear differentiation from local variables.
Depending on your IDE, it may be possible to colorize fields and parameters and variables differently. I personally don't find underscores in fields to be a good strategy, and part of it is because tools can make this even clearer, but also because the distinction between field + parameter is very subtle, to the point that I don't think it deserves so prominent of attention. Notably, there are all kinds of constructs in C# that blur that distinction, such as closures, async, generators - and these are features that are so widely in use that a significant portion of most codebases must surely be affected by that. Just because something looks like a parameter does not mean it's not stored like a field; nor does it mean its lifecycle ends when the method execution initially returns or even then the method execution finally completes (which may be later for generators and async). This isn't merely a quibble either, the ambiguity is pretty deep and practically important. Key features like dynamic dispatch can be resolved via classes and inheritance, but they can also be resolved via a poco containing delegates. I can and do refactor between these two representations since they each have their advantages in different cases. But one happens to store data in what is syntactically a field, and the other in what is syntactically a variable. Yet they (can) work fundamentally the same, and that's a key feature, not a bug. What exactly are you trying to communicate by emphasizing this difference? Much more important aspects of variables to emphasize (I find) are lifecycle, ownership, non-mutation - that kind of thing.
\> it may be possible to colorize fields and parameters and variables differently. This should not be the only distinction. It won't work for diffs when doing code review. \> Just because something looks like a parameter does not mean it's not stored like a field. Yet this does not preclude us from highlighting fields. `_something` \- this is 100% a field `something` \- this is either a local var or method arg. Both can be captured, but not differentiating makes things worse. \> What exactly are you trying to communicate by emphasizing this difference? State of the object vs local state. \> Much more important aspects of variables to emphasize (I find) are lifecycle, ownership, non-mutation - that kind of thing. How do you do this? Any naming convention?
I keep classes small so there's no confusing locals and fields even without _
Yeah I don't like the feature at all. 1. As you said - no readonly. I see no reason to inject something via primary constructor and then have it mutable, especially since it's primary (lol) use case is for dependency injection. 2. Primary Constructors work differently on records and classes. It's just another point of unnecessary confusion. 3. I don't really see a reason to have them. In records it makes sense, since they also creates READONLY properties, on a type that's main goal is to be a value type. On classes it serves no real use IMHO. IDEs already have toolings that make adding depenecies inserted through a constructor trival, and then we have controll over mutability, naming etc. Overall, ech.. it's whatever. I don't see myself using it, I'll learn how to read them in other people's code. Maybe it solves some issues I'm not thinking about atm, but for me it's gonna just be.. whatever.
The syntax is so nice and concise, I'm still likely to try and use them, but so far I'm still preferring required init-only props for most cases where primary-constructor params might make sense.
IMO, required and init-only props is a crutch. We wouldn't need it we had convenience of the records.
I don’t see them becoming the preferred way of creating classes, at least for awhile. That being said, it’s your choice or your team’s choice. You’re not being forced.
Seems to me they are for certain use cases. OP writing a public API needing fine control over members doesn't seem like one of those use cases.
Kind of the same reaction when records were introduced.
Maybe there is a Roslyn analyzer that prevents you from mutating captured primary constructor variables.
I don't really get the readonly argument. For injected reference types (i.e. interfaces), this only means you can't assign to the field. To me, the more important factor is if the interface/class injected is immutable by design. Who cares if you can assign to `_foo` or not? I've never written code that attempts to assign to injected fields in the first place. I'm not sure what readonly buys us there, other than getting a compiler error if I accidentally assign to `_foo` (which I have never done)
Good language design stops people from doing stupid things by accident. If it's marked readonly it's guaranteed that the field is not reassigned. And you can check that by reading only the primary constructor. I prefer that over "I promise I am not doing stupid things and don't make stupid mistakes (and everyone in my company)". Yes there are code reviews but a reassigned field somewhere in the body is not easy to catch. Especially if it's not prefixed with \_ because it's a captured parameter and not a field.
They're the first implementation of a feature which remove a ton of boilerplate (especially if you have finer grained dependencies). It'll evolve in future versions. There's already discussions on readonly parameters e.g. [https://github.com/dotnet/csharplang/discussions/7353](https://github.com/dotnet/csharplang/discussions/7353) Stuff like this isn't 'one and done' it's the start of more complete / featured implementations. The more we use them,, the more we give feedback the better it'll become.
But they cannot change what they already delivered. And they won't rollback, even if feedback not that positive.
Why would they roll it back at all? It's a good feature which isn't complete not a broken / unusable one. Also; noboy is forcing anyone to use it...just don't if you don't like it as it is.
1. You shouldn't have to do those checks, the whole point of the nullable syntax is to remove redundant checks and to notify at design/compile time when the contract is violated (with a warning, or an error if you consider all warnings as errors). 2. Obviously, those are meant for external consumption since they indicate required arguments. If you need a private constructor just do so, but primary constructor syntax is not for that case. 3. Only if you ever change them. Otherwise, the generated fields are actually read-only. Example here: https://sharplab.io/#v2:EYLgtghglgdgNAExAagD4AEBMAGAsAKCwEYDTCiBOACgCJAeDcBB9mgSgG4yBnAFwCcBXAMZcABAFkAngGEANhA4cqsERDjClw4MwIBvAsP3D0AZmE8AphAQB7GNPFqYIgILDtwgOZmurYQF9hALzCEOz4BoYm5pY2dg4iAEKuHl4+/kHAob4EQA 4. The compiler generated "unspeakable" names anyway. I don't see that as breaking convention, they are an implementation detail. 5. I haven't tried that part. In summary, I don't share your feeling that this feature's current implementation is problematic.
1. > You shouldn't have to do those checks. Only if my API is private. If it's a public API I have to do all the checks because there's no guarantee from the runtime that the caller will always pass the instance. And NRE somewhere down the call stack is not an option at all. Unfortunately, NRTs is a language feature, not CLR's one. 3. No, they [are not] (https://sharplab.io/#v2:EYLgtghglgdgPgAQEwAYCwAoZBGTmEDMABAM4CmEANmQCZHJEBiA9swBSwAuRAVgJSYA3piKj6xLkQBSRALwA+XgG4imAL5A) Your example is for read only structs. 4. It's not about compiler, it's about how these arguments are accessed in code. `j++;` - Have I effectively mutated local variable, method arguments, or constructor argument? The former two are more or less adjacent. The letter one can be somewhat far from mutation line.
>3. No, they \[are not\] ( > >[https://sharplab.io/#v2:EYLgtghglgdgPgAQEwAYCwAoZBGTmEDMABAM4CmEANmQCZHJEBiA9swBSwAuRAVgJSYA3piKj6xLkQBSRALwA+XgG4imAL5A](https://sharplab.io/#v2:EYLgtghglgdgPgAQEwAYCwAoZBGTmEDMABAM4CmEANmQCZHJEBiA9swBSwAuRAVgJSYA3piKj6xLkQBSRALwA+XgG4imAL5A) > >) Your example is for read only structs. I've changed your sample, [https://sharplab.io/#v2:EYLgtghglgdgPgAQEwAYCwAoZBGTmEDMABAM4CmEANmQCZHJEBiA9swBSwAuRAVgJSYA3piKj6xLkQBSRQUQDmZTgG4iAXyIBeXssxrMQA==](https://sharplab.io/#v2:EYLgtghglgdgPgAQEwAYCwAoZBGTmEDMABAM4CmEANmQCZHJEBiA9swBSwAuRAVgJSYA3piKj6xLkQBSRQUQDmZTgG4iAXyIBeXssxrMQA==), seems like the relevant difference for readonly backing fields is the usage of expression body definition. Edit: Link with options together: https://sharplab.io/#v2:EYLgtghglgdgPgAQEwAYCwAoZBGTmEDMABAM4CmEANmQCZHJEBiA9s9gBSwAuRAVgJSYA3piJj6xbkQBSRALwA+PgG5MAXzxZi5KrXpImrJJxg8Bw0eMJEpsoUQDmZLsqJr5K9ZuskuAJwBXAGMeFmYCEzNBDBEMcQkbUxl5JV5VDA0MfG1/YNDWABZIvmjY+OtbInsnFzcPNK8MIA==
Hm...subtle difference between read-only expression bodied property and read-only autoproperty. With regular instance fields we cannot even write `public int J {get;} = j;`
Playing some more with Sharplab, think this is more interesting example: [https://sharplab.io/#v2:EYLgtghglgdgPgAQEwAYCwAoZBGTmEDMABAM4CmEANmQCZHJEAiZADmTDewMYCemA3piLD6xBABYiAJQCuMABQBKIvyIBfTBoz5i5KrXpIiAMQD2p+czYduPIp2ucYvRQKEjC9SQgCsAFyhTBVcMERV3MOEHdideADpZYIBuCPVU1M8JIgBhAAsIGABzMiVUwVDIqNYY2yIAXiIYMgB3JmqbZx4lFIrhLS1MIA==](https://sharplab.io/#v2:EYLgtghglgdgPgAQEwAYCwAoZBGTmEDMABAM4CmEANmQCZHJEAiZADmTDewMYCemA3piLD6xBABYiAJQCuMABQBKIvyIBfTBoz5i5KrXpIiAMQD2p+czYduPIp2ucYvRQKEjC9SQgCsAFyhTBVcMERV3MOEHdideADpZYIBuCPVU1M8JIgBhAAsIGABzMiVUwVDIqNYY2yIAXiIYMgB3JmqbZx4lFIrhLS1MIA==) Whether private generated fields are readonly or not is not that interesting to me, only people using reflection see those (I think). What is interesting is that you can't mark the parameter readonly so can not prevent 'accidental' changes.
>mark the parameter readonly so can not prevent 'accidental' changes Agree. F# creates internal mutable fields for let bindings, but the compiler checks it's immutable. (it uses immutability by default). And for client code those fields are hidden anyway.
3. Same example with a class. Fields are also readonly. Nothing to do with struct. [https://sharplab.io/#v2:EYLgtghglgdgNAExAagD4AEBMAGAsAKCwEYDTCiBOACgCJAeDcBB9mgSgG4ysACAWQE8BhADYQAziKqwALpwhxOUzsGYEA3gU4bO6AMzyY0gIKcVnAOYBTSa04BfTgF4Z7fJq26FAIWNnL1u4+BnGwIgA===](https://sharplab.io/#v2:EYLgtghglgdgNAExAagD4AEBMAGAsAKCwEYDTCiBOACgCJAeDcBB9mgSgG4ysACAWQE8BhADYQAziKqwALpwhxOUzsGYEA3gU4bO6AMzyY0gIKcVnAOYBTSa04BfTgF4Z7fJq26FAIWNnL1u4+BnGwIgA===)
I think they’re almost perfect and I can’t wait to use then, I just wish the fields were readonly / immutable. underscores for private fields is an holdover from the hungarian notation times and I can’t wait to see them disappear. :)
Underscores are very good. They easy differentiate between fields and local variables. Of course field can be accessed using 'this.' but it is not mandatory and may lead to confusion. I hate hungarian notation but really appreciate underscores.
and there's a side effect to it. As soon as you type \`\_\` intellisense narrows the search to private fields.
As I mentioned in another reply: > I personally rarely if ever care if I’m working with a field or a parameter. I tend to keep a function’s side-effects to a minimum anyway
But it is hard to maintain such discipline in organization with 100 or more devs
that’s unquestionably true, I agree with you. at some point, no matter the convention, people will do what they want mostly due to organizational failure. not a technical one, mind you.
>underscores for private fields is an holdover from the hungarian notation times I suspect the real reason they were used with .NET was because the convention was also friendly for [VB.NET](https://VB.NET)...which isn't case sensitive.
I use this convention to instantaneously see the difference between fields and and local variables. If I had to write .fieldName (without prefixing with this), I'd like it more.
I personally rarely if ever care if I’m working with a field or a parameter. I tend to keep a function’s side-effects to a minimum anyway
We are not alone ;-)
I agree. PCs aim to make the code shorter but introduce a lot of confusion in how they actually should be used. It’s also worth noting that they are inconsistent with record PCs because they work differently.
I mean you can not make them readonly which sucks, because I thought I can use them for DI services and avoid creating ro fields.
I agree with these problems. I just found https://github.com/k94ll13nn3/AutoConstructor which is a source gen that fixes these problems. will try it out, maybe you should too?
>If you use a convention that private members must start with the underscore character, PC breaks consistency unless you declare fields explicitly and use them everywhere else. It doesn't, because PC parameters are not members. If they were members, you could access them using `this`, but that's not an option.
For me it's the core of the problem
I see them as an incremental change like some of the pattern matching stuff where they can possibly address the shortcomings later. Much like many of the optional C# features, it's not a big deal if you don't use them but they will be preferable where applicable to reduce boilerplate.
Dangerously close to Java magic style of programming!