"The readonly Keyword Is Useless!" | Code Cop
2024 ж. 25 Ақп.
34 683 Рет қаралды
Use code MODULAR20 and get 20% off the brand new "Deep Dive into Modular Monoliths" course on Dometrain: dometrain.com/course/deep-div...
Become a Patreon and get special perks: / nickchapsas
Hello, everybody, I'm Nick, and in this episode of Code Cop I will talk about the readonly keyword and why I think primary constructors weren't really thought out well enough for launch.
Workshops: bit.ly/nickworkshops
Don't forget to comment, like and subscribe :)
Social Media:
Follow me on GitHub: github.com/Elfocrash
Follow me on Twitter: / nickchapsas
Connect on LinkedIn: / nick-chapsas
Keep coding merch: keepcoding.shop
#csharp #dotnet
The problem with the last example of manually creating the readonly field by setting it to the primary constructor parameter, is that the primary constructor parameter is STILL in scope to be used alongside the readonly field.
Not if it has the same name
@@swozzaresexactly, but the last example explicitly doesn't use the same name. (I'm sorry Nick, usually you are very thorough, but I think here you made some mistakes.) Using the same name would in the other have probably break one naming convention, either for the parameter or for the field. Anyway, I specifically think the last example is problematic, because it should show us a safer way, bit introduces this risk.
It at least throws a compiler warning, so if you don't allow warnings, or if you set CS9124, or go nuts like me and treat all warnings as errors, then you won't be able to build with that - can't even read from it.
Hmm looking back at this, I thought Rider uses it's own analyzers or so, but it looks like there is a bit of a slowness problem or so. Nick changes the code to use the field with the underscore at 8:44, but after this the yellow line remains. This is strange. Roslyn actually allows this usage since the beginning and I have written several classes like this meanwhile. I already commented about this yesterday, but at least in VS and VSCode I have no issues with here, it really only warns when actually assigning to the field and using the parameter directly at the same time. And Mads Torgersen has also explained this very well in demos.
Oh wow, that's surprising. I had been preferring that last method but this definitely has me reconsidering.
6:43 remember - that "everyone" also includes yourself in six months. And that developer needs all the help they can get
"six months"? Sometimes it's six weeks or six days.
@@OhhCrapGuy I had to switch from the .NET WPF app project only I was working on to Ruby on Rails for a week due to staff shortage when I returned I felt like I entered uncharted territory with code I had never seen before even though I had coded it 😂. I am glad for the hints I left for myself
Thanks for the shout out @nick. Yours truely... JohnB :)
i was so excited about primary ctors until i used them. For the reason described in your video. This was a major oversight by the C# team.
I love this new syntax but do not understand why it wasn't implemented as a readonly field 😭
Well, it is simple. You don't need language support for making these fields read only. Just use any static analysis tool available to you. Jike Sonar, Roslyn or CodeCop.
@@jantajovThat doesn't make any sense. By that argument, the readonly keyword should be removed from the language altogether
@@SkyyySi can you tell me what do you think readonly keyword does? Don't google it, just tell me what you think it does.
@@jantajov plugins are great, but we are talking of 'raw' C# here, native support.
true. the default should be readonly. if you REALLY need a DI parameter that is NOT readonly - simply use the old ctor syntax. but it would collide with the standard c# syntax. void f(int x) and x can be modified inside the scope of f (without affecting the original input). Worse void f(SomwClass o) and you can both assign a new value to o or call methods of o that mutate it.
I prefer to just have the readonly field defined explicitly. It is easier for me to understand what is going on when things are not hidden.
I was thinking that as well. Seems like the trend for a while has been to try to hide boilerplate, rather than simply abstract it, but I prefer to see it so I know what the actual code is. Features like this are less readable.
Agreed, but we seem to be in the minority. I really don't like all of these new "implied" things. Explicit code is easier to track, especially when you didn't write it, or when you wrote it a long time ago.
@@TicoCryptonaut exactly my thoughts!
It's definitely useful to know about this feature, but I don't see it as an issue. Who assigns a value to arguments passed through a constructor anyway? Doing so is often a sign of bad coding practices or a beginner's mistake. Also, the fact that the field is no longer 'readonly' with the primary constructor didn't exactly make me eager to assign anything to it. However, it does save me from having to write the same variable three times: in the constructor's parameters, within the constructor's body, and in the field declaration
If the only thing stopping you from assigning a new value to an argument is the presence of readonly, there is something wrong with you. There are no such restrictions in python, but developers must know when to modify a variable and when not to do so
@@neverovnikita if I can prevent bad coding practices or beginner mistakes with a single keyword - tell me why I shouldn't do just that? I also never understand the "less keystrokes is better" argument (having to type the name of a variable multiple times). I'm an advocate of verbosity beating char count any day - because I can remove ambiguity with more chars. I'd much rather see a clearly defined constructor assigning to readonly fields for dependencies, than assuming the behaviour of params to a primary constructor. Another classic case of "just because you can, doesn't mean you should" when it comes to refactoring recommendations
@@neverovnikitaI also prefer to "know" or "assume" less, and enforce more. As soon as I expect a dev to just "know" not to mess with a dependency field, I also now know that it's possible - and Murphy's law says if something can happen, it probably will...
@@iantabron I've been thinking about scenarios where a beginner might feel the need to assign a new value to an argument, and I couldn't really come up with a case. I'm sure if a newbie wanted to assign null or a new value, they could just as easily remove 'readonly' or forget to specify it. When it comes to verbosity, a developer is statistically more likely to make a mistake when they have to mention the same variable three times than just once. A developer might forget to assign a value to a field or mix them up if the types are the same. I agree that verbosity is important, but only when tackling a new task. In this case, it's a standard operation that doesn't require much attention. That's why they introduced top-level programs recently, and now nobody writes `class Program { void Main()...` because it doesn't add any additional information. Just like how you're comfortably using auto-properties instead of defining a backing field and feel fine about it, most importantly, making fewer mistakes
@@neverovnikita I don't necessarily disagree with anything you've said specifically - moreso the general idea that "less is more" just doesn't sit right with me. Using readonly indicates my intention as the originator of the code, just as access modifiers do - and the argument of "someone can change it if they really wanted to" doesn't stack up in my opinion. It is far easier for a developer to just assign to a property that shouldn't be by assumption, than to remove a keyword - particularly one as clear as readonly. It forces a dev to think before doing, and would more easily fail a PR than a simple assignment further down in the code too.
So why is there no support for readonly in constructors? It exists in Typescript and it's great. Seems like a hugely missed chance. EDIT: They should just allow for `readonly ` as a parameter to be honest.
I assume we already have a keyword "in" for the functions. maybe they can add "in" to primary constructors instead of readonly.
From the github discussion it seems the reason was "it's better to ship unfinished feature asap than finished feature in a few years" which I don't think I personally really agree with...
@@LeMustache This entire issue can be sidestepped by a multitude of analyzers. It's by no means a perfect solution, but waiting (at least) an entire year just because you don't want to use an analyzer feels like a missed opportunity. I can't speak for anybody else, but I've swapped over to primary constructors for DI and I'm very happy.
@@modernkennnern I'd rather have them focused on issues that were supposed to be released 4 versions back than come up with half baked feature with arguable value and rushed development.
With the final solution "dependent" is still available for unintended use in other methods. I'd like to either have the readonly modifier or dont have the constructor parameter automatically assigned to a field if the names match.
Is it actually available? In the lowered code at 9:05 "dependent" is only set up as a local in the generated constructor. But maybe this is only because the original code doesn't attempt to use it outside of the field assignment?
@@TazG2000 Yes. It's not available becaus it' not used. If you'd use it in a method it would become available again.
Do I miss it? Yes Do I think it's a reason not to use primary contructors? No In my experience, if some Jr dev wants to do something wrong, they would just remove the readonly and mutate the property anyway. It's a pretty low fence to jump over. It surelly doesn't add _that_ much security.
A removed readonly keyword is more likely to be noticed in a code review, though
You can make this argument on just about any compiler feature. Why not use a fully dynamic language in the first place if the go-to excuse boils down to "just don't make any mistakes".
for me, the readonly keyword also transports intend and therefore acts as a form of documentation. If someone removes it intentionally, that someone better have a damn good explanation for that. If the readonly isn't there, that hint is missing. In my eyes a significant loss in terms of documenting intend directly with code.
Nobody will remove a readonly keyword. They will simply don't put it on newer implementations.
Thank you for these cop series. They are amazing!
I just started learning C#, so I am not familiar with all the ins and outs. My IDE has been suggesting to use primary constructors, and when I used them, I ran into some weird behaviors that I didn't understand. I'll stick with the normal constructors for now. I am slowly falling in love with the language.
Maybe also good to notice that in many cases you really want to consider using a record (which is actually a record class) instead, or even a record struct sometimes. Normal classes with primary constructors are really for those situations where you want your class to have actual methods with logic as well. The typical OOP thing. But if you really just need to represent a set of values and you do not need to modify these, you probably want a record instead.
@@jongeduard Thanks for the clarification, that makes a lot of sense!
@@Stoney_EagleYou're welcome.
I have Visual Studio Preview 17.10.0 Preview 1.0 installed and we're now offered two refactorings Use primary constructor Use primary constructor (and remove fields) The first of which does now leave the private readonly ..... variable definition intact 🙂
Exactly. No need for the Preview at all. This is already in the Roslyn project for months now, and therefore in regular VS as well, but also in VS Code. 8:42 So this what Nick sees has to be a Rider warning, not a compiler warning. Preserving readonly fields is fully valid and accepted practice. The only situation where you get an actual warning happens when you assign your parameter both to a field also use it directly. Mads Torgersen has also showed some demos about this in which he explained this very well.
They should have catered towards the DI side more initially. Having the default be readonly, and perhaps have some compiler attribute that makes it mutable if specified. Like PrimConstructor([MutablePrimaryConstructorParameter] IDependent dependent)
This would never happen because it's not backwards compatible. The cat's now out of the bag and people are using it as-is. They should just simply introduce the `readonly` prefix on the parameter. I bet it was slated but they ran out of time.
As much as I agree with this in general, the paradigm in c# is default mutability, changing it for one feature would be confusing drive people nuts and changing it globally would break backwards compatibility severely. Unfortunately we are probably stuck with default mutability for the lifetime of c#. It would require a big break on the par with creation .NET core becoming separate from .NET framework again. If this were to have been done that was the time but default readonly wasn’t as in vogue back then and the .net team was much more conservative about the types of changes they were making as they wanted to actually convert framework users. Frankly a bigger change I would like to see is just less verbosity in general. When c# was created, like people valued explicit tags so you get declarations where an object has like 4 key words. Which is kinda bonkers as most of them could be defaulted to sensible values. This is almost more of a cultural shift than a feature one as there are already reasonable defaults. For instance methods and fields are private by default, why type the word… and classes are internal by default.
I am using this feature with Mediator freely without any issues, so not sure about the point of handlers.
I had issue and If i can recall the problem was to inject IMeditor itself. It was null. I do not remember precisely anymore tbh. When you see null exception within a handler remember this.
Seems like the primary constructor feature has not been thought of thoroughly by the developers of the feature. It's not "baked". This video sums up the reasons why we are hesitant to adopt this in our code. Also there's a naming convention issue here in my opinion. While fields have a "_" prefix which makes it clear in the code that I'm using a shared resource which is not owned by the method, using primary ctors prevents this - which is a problem for me. In fact, I would like to have a feature, in the future, to have DI directly on the fields without using a ctor at all. Similarly to how Beans are used in java. Just have a: Class { [Inject] private readonly IDependency _dependency; } Or - Use a primary ctor, but have the option to generate fields and don't allow methods to use the primary ctor's parameter directly, but have them only use the readonly field assigned. This gives the advantage of boilerplate code removed (ctor which only assigns fields), but keeps the safety and readability.
I use the primary constructors with the "manual" readonly fields everywhere now and it works just fine. Yes, there's an extra line for each dependency, but there's still less lines of code than with the standard constructor and the end result is the same. Not sure what the comment in the video about MediatR handlers is about, every single MediatR handler I have is using primary constructors and "manual" readonly fields.
I've built a couple new projects in .NET 8 lately but I haven't used primary ctors. The syntax makes creating new POCOs super easy (a.k.a. records) allowing me to slap multiple one-liners into one file, nice and arranged. With ctor-injected dependencies I got used to just ignoring those - you're also not supposed to have too many dependencies in a class anyway so it's not a big deal. Maybe if they add readonly to primary ctors I'd start using them.
How to validate parameters of primary constructor 🤨?
public class BankAccount(string accountID, string owner) { public string AccountID { get; } = ValidAccountNumber(accountID) ? accountID : throw new ArgumentException("Invalid account number", nameof(accountID)); public string Owner { get; } = string.IsNullOrWhiteSpace(owner) ? throw new ArgumentException("Owner name cannot be empty", nameof(owner)) : owner; public override string ToString() => $"Account ID: {AccountID}, Owner: {Owner}"; public static bool ValidAccountNumber(string accountID) => accountID?.Length == 10 && accountID.All(c => char.IsDigit(c)); }
If you need constructor with body - just use constructor with body
knew what you were about to say as soon as I saw "readonly" on a Code Cop video notification
Feels a bit like an oversight from the design process of primary constructors. Why not allowing to specify access modifiers on it?
What's the official statement from Mads Torgersen (and the C# team in general) on omitting the readonly keyword in the param definition for primary constructors? It's quite interesting that they either didn't catch this (which is unlikely) or simply ignored it (more likely).
Two of us at my company use Rider. I spoke with the junior developer who is also using Rider and showed him this problem and even he said, “Yeah we should just disable that suggestion to convert constructors to primary ones.” From a junior developer, he was able to see the problem just as plainly. We both agreed that this feature is not ready for general use and will stick with our traditional constructors. It is shocking that Microsoft didn’t allow for the readonly keyword on primary constructors.
I don't like primary constructors in the first place. I think the little bit of boilerplate is still more readable than this, in my eyes, good for nothing feature. Apart from that, the readonly keyword should be made usable in the primary constructor. I think it's crucial that developers who want to use primary constructors are able to define whether the generated backing field is mutable or not.
Exactly. They look 'cool' in examples with just 1 parameter, but in the reality with at least 5 DI members it will be so hard to read.
Use the last option you provided but remove the underscore from the private field name. The closure will ensure only the field is accessible within the class methods, which means it can't be modified since it's readonly.
I'm with Nick here. I'm not using this feature, as much as I want to, until I can mark the parameters as readonly. And yeah, to Nick's question at the end, I'm down with having it in the constructor parameters. I like how this flows in TypeScript. I think it should work the same way with primary constructors.
Thank you so much for your awesome tutorials Nick!
Your argument for fields vs. properties is actually exactly right. There is literally no reason to have a property when you don't have any getter and setter behavior and both are public. You have no safeguards in place, all you're doing is wrapping the field access. I wouldn't use properties in these cases, if it wasn't for the fact that it is sort of required for my work. C# has the additional benefit that, while coding, properties and public fields act exactly the same. Even more reason to not use basic auto-properties, since you can just add them later without breaking anything. Changing visibility will break dependent code either way. I think the same goes for primary constructors. They make your code more readable and, as you show, you can still use readonly with them if you really want to. They are _private_ fields, and you should know that messing around with class internals when you don't know exactly what you are doing is rarely a good idea. The only thing you do really miss is the very minor memory optimization. Also, MediatR DI works just fine for me with primary constructors.
"you can just add them later without breaking anything" sure, if you recompile the code that uses it. But if you just replace the old binary (with field) with the new binary (with property), then it *will* break code from other binaries at runtime.
Reflection would be one reason to need properties even when they don't do anything - I don't remember off the top of my head which ones, but I've run into serializers or flat file parsers or things like that, where it recognizes properties but not fields.
@@JoeEnos Also fair enough. That has less to do with property vs. field, and more so with implementation details and their assumptions due to conventions.
@@vyrp Fair enough. That would mostly apply to NuGet Packages and non-monolithic solutions though.
Have to correct myself on the MediatR part. Started to run into DI issues, though only under certain conditions. Seems to be related to which assembly the service originates from.
I realized the same thing when trying out the new refactor suggestions from Resharper. It's almost a useful way to remove boilerplate, but all of my DI tends to be readonly, and I would mainly be using the primary constructor feature to replace my DI. So I end up not using this very often.
Primary ctors with an analyzer works nicely.
The last approach! primary constructors + readonly field, BTW you can name the field exactly like the parameter, the compiler will know to distinguish between them.
I'd love to see support for accessors and keywords like "required" for primary constructor parameters. I think the point with roslyn analyzers is, it would be an required dependency for a very central concept that needs to be maintained over time and developers has to know that something implicitely happens behind the scenes. So from my perspective it's a bit hidden and thus hard to get.
We use read only properties for our injected dependencies and setting them from the primary constructor does not result in a suggestion to remove them.
I'm not a big fan of a primary constructor. What if I want 2 different constructors? I should be able to extend primary into secondaries.
You can easily do that.
One additional thing you can do is name the primary ctor same name as the `private readonly` and it will not let you overwrite. but it's a bit awkward looking. would be much better if we could just specify "readonly" in the primary ctor... maybe C# 13 void Main() { new MyService(new IDep()).DoSomething(); } public class IDep { public void Do() { "dump".Dump(); } } public class MyService(IDep _dep) { private readonly IDep _dep = _dep; public void DoSomething() { _dep.Do(); } }
I appreciate that you get a compiler warning when you set the readonly field value to the primary constructor parameter and also try to use it in a regular method. I personally treat all warnings as errors so it won't build if you try to do that, but you could also just do CS9124 for this specific one. But I would have appreciated more if they would have had the readonly part done in this first release.
The example code in the LI post wouldn't compile anyway as there's a typo. The readonly property is of type IDepedent 🙃
Excellent point about losing ReadOnly. Makes me think of when a boss of mine said he wanted to shorten my review time and asked if that would impact my review. I told him he could cut my review time to zero if he wanted. He seemed surprised asked really?!? I said yes, just tell the engineers whose work I was reviewing not to make any mistakes.
I was initially super hyped for Primary Constructors, precisely because less boilerplate is always nice to see, but when it finally got released, it was such a disappointment. I could probably deal with it if it were _only_ that it wasn't readonly, because while that's not necessarily "best practice", it's not the end of the world, you just have to be a bit more careful when writing and reviewing code. But for me, it's the fact that it behaves fundamentally differently to a normal constructor, which means your IDE is potentially suggesting (quite aggressively I might add) that you introduce breaking changes into your code.
I think I'm good with Nicks solution here. You can even set this as a snippet in your choice of IDE. So when you do the refactor, this snippet is what it produces.
I agree that making primary constructor arguments read-only would be safer, still, I would point that re-assigning _any_ input arguments may be considered bad (and potentially dangerous) practice. Yet we don't usually declare our method arguments as readonly - we just develop a habit to not do this. So what's the difference about the primary constructors? Just do not ever reassign injected arguments (add an analyzer if you want be sure for 100%).
Primary Constructors needed more time in the oven, at least before suggesting them by default, Not that I see a clear path forward that won't anger users. Maybe a generic Readonly or Injected class marker Maybe an unused symbol or short keyword Maybe re-thinking ctor DI. Maybe constructors should just be for fields/initialization and services should be configured in a different way. Maybe everything in the ctor can be readonly. When do you pass things to a class that you want to mutate?
Hi Nick, generally you are right and such fields (and probably primary parameters in the first place) should be readonly. But I also think that don't use readonly in very small classes that have ivory a few lines (like you example) is not a big deal, and you probably use this rule too strict. Also, you usually completely own the class and can make sure the field is written to, when though it is technically possible. Therefore your example with a public field instead of a property is actually a bad example. because Public field, everyone can write to, but the primary argument is protected and cannot be directly accessed internally.
7:34 “it’s amazing, it’s great!” 😂
I used to disagree that the default should be readonly, but I always assign them to fields or properties anyway, and even if the "constructor variable" was readonly, nothing would/should prevent me from assigning it to a normal field if I need to be able to change the value/ref. The "in" keyword would, but I'm not sure if that works with primary constructors.
Wow, ive just setup a new service and used primary constructors everywhere..none of us knew this 😂.. time to go back!
Don't do that. It's fine.
This is a non-issue. Just install an analyzer instead it if you're worried.
dont rollback, this is really a non-issue dispite the backlash ...
Did anyone reassign a dependency? I bet they didn't.
After sleeping on it, it sounds pretty silly for me to change it all back
I haven't had the chance to use primary constructors yet, but wouldn't the "in" keyword fix this issue and make the argument readonly? Unless it doesn't work with DI, which is what I think the problem is, unsure though
To me, the "solution" to the problem where you add another field that is readonly field and initialize from the primary constructor injected field makes the situation much worse because now you have two private properties available in your class - dependent and _dependent, the former one which is still editable. It would be easy to reference both of them in your class. While I agree that the primary constructor should have supported readonly injected properties, the benefit of massively reduced lines of codes outweighs the risk. An injected property should never be modified, but I have never seen a reason for anyone to even want to do that. And if you have developer who would do that, readonly keywords are not going to protect your code base from the damage that is coming.
The simplest answer is, use the primary constructor if the use case does not require read only fields. If you do need it in a primary constructor, you'll need to manually re-introduce the field in the way demonstrated in the video. If you don't like the compiler graying out the manually added read only field, you can access the new read only field via a property. But then you haven't gained anything in the reduction of boilerplate code.
This is my example: private readonly int _dependent = dependent; private int Dependent => _dependent;
I make as much as possible immutable, that also includes making fields readonly. I also suppress the suggestion about the primary constructor because of that. I encountered a number of nasty bugs in the past because something quietly mutated things I was not expecting. I wish there was something like Java’s “final” or Scala’s “val” to also make variables immutable. “Const” only works for things that can be evaluated at compile time.
hey nick great video. If you can make a video about FastEndpoint that will be great i'm using this framework in a lot of project and i like to see your opinion
I kinda like the primary ctors, but I find myself having to refactor to a normal ctor every now and again if I need some logic in the ctor, not great. I must say that the last example you showed, where you still had a readonly field but initialized it to the ctor param looked nice. I think I'd rather have that and only be able to use primary ctor params to initialize fields actually. Imagine if I then could just add a block for the ctor somewhere if I need some logic without having to completely revert to the old way...
I like that language is evolving and getting new features, but sometimes those are too much. We are getting more features, which shorten written code, but not always in a good way. I think readability is most important and code with primary constructors should be only for model classes. When we need to modify or use fields in our class, then fields are the way to go. We need to remember, that we will look at that code in the future and we don't need to guess, where what was declared or initiated. Debugging this shorten code would be a nightmare.
Can't you just add roslyn analyzer that would catch assigment to primary constructor parameters? I don't think it is something you need language support to achieve. Or StyleCop rule or any other static analyzer tool.
And if some other team starts working on your code without a static analyser years later? How would they know your intention? Explicit is always better than implicit.
@@Gattuser Roslyn analyzer is part of C# compiler. It would throw error during compilation. It would also catch cases where you forgot to use readonly keyword, in future when they added support for it.
@jantajov well, for sure, I agree. But, as of now, there is a way to write your code only relaying on the language support, and it will be 100% clear and explicit (the old way) Or, alternatively, use this syntactic sugar with a price of adding another dependency on an extra tool. I just think that this logic (philosophy, if you wish) is flawed
@@Gattuser There isn't. Primary constructors cannot have read only parameters.
I just can't understand why C# guys haven't done this in the same way it is done in Typescript where you can add private/public/protected/readonly modifiers to constructor parameters... it's just so natural syntax for this.
Hello! I am new to c#. I have never used "var" for declaration afraiding of forgeting what is the "container" type of the object, because sometimes there is an implicit casting behind the scenes, and therefore i just do not like to use var. My thought is that i do not like the feature you are showing here for a similar reason. The reason is that i might forget about the entire field at a code review, so i am not even upset about the "readonly" thing, i just do not like the entire feature with a different point of view. I would never recommend to use it regardless of the "readonly" keyword
I don't like primary constructors. For classes with many properties this becomes a mess when you need private fields, readonly properties and some fields in primary constructor. I like more init properties which might help you to avoid class constructor but you have one place to look for all class properties/fields
I really like the idea of primary constructors for DI (less boilerplate), but without supports readonly fields I just can't see myself using them. And I don't particularly want to use them elsewhere either, as it would seem weird to use primary constructors in some parts of the codebase and then not use them in other parts.
By this reasoning you can’t ever use new syntax, because other parts of your code base is not using it 😅
@@ryan-heath You might be shocked to hear this, but it is actually possible to go back and refactor old code when new language- and runtime features come out.
@@MZZenyl I know, and I hate it. 😉 If there is no reason to change code, dont change the code! Just syntax change is not a valid reason in my book.
@@ryan-heath I'd rather keep codebases modern, as to not eventually stand with an utterly outdated mess and largely in need of a full rewrite to integrate with modern systems. But reasonable minds can differ.
@@MZZenyl Sure, but I have seen this going wrong too many times, introducing subtle bugs in code that was working fine in the first place ...
I have noticed that if you use primary constructors to pass in an EFCore DBContext, the context will be null. Maybe I am doing something wrong, but changing it to a constructor works fine. I think primary constructors might be a good thing, but if you use naming conventions where an underscore is prepended to class level variables, then your constructor is starting to look odd. public class Person(string _firatName, string _lastName) I think we will see more iterations of this feature before it is a true replacement for normal constructors.
Injecting DbContext with primary constructor works for me
I don't think I'll ever see a practical use for primary constructors specifically because they just generate mutable fields. Most of the time, I will tend to reserve constructor arguments for resource acquisition in order for the object to be in a valid initialized state and mutable fields tend to go against this principle unless I add a bunch of guards against this in all other code that consumes said fields.
to mee it just looks like we are moving the boilerplate from the constructor to the class level, doesn't seem like worth the trouble for DI, for properties it is another matter
I prefer to use the keywords explicitly. For anyone reading, it is clear what the intention was for that property or field. I think if they allowed readonly keyword to be added via the primary ctor, this would help, but I'd probably still prefer the explicit declaration, again, for the same reasons.
Honestly I feel like primary constructor parameters should be read-only by default and if you need to mutate them or whatever then you should write your own additional code Make it an incompatible, breaking change between versions but I think only then primary ctors will have a place in production code for me
Whilst I get the point, has anyone ever been bitten because a field wasn't marked as readonly? I can't think of a single time where I've ever been even slightly tempted to reassign the value of any injected dependency. And I can't think of any code where anyone else would want to. I get the safety point, but surely this is overblown?
This is similar to how positional records don't support the required keyword. The IDE can convert your record into a nice one line... But strips out the 'required"
Personally I would like it would allow syntax like class MyClass(readonly IDependent dependent). And that's maybe relatively easy to add into C# lang spec and compiler.
This one triggered the "i feel uncomfortable about this" so i just fired VS to confirm something. And yeah... it was just as i thought, even though it probably sounds obvious. The author makes the case that readonly is pointless because your class should know better than to change it. Nothing stops your class CAN pass BY REF to some other "clueless" class that will mutate it though. But you can't pass readonly's by ref (outside the constructor)... It's a good example of "it's there to stop you from shooting yourself in the foot".
Some interesting (and worrying) reads in the comments already. Worrying that so many people are using the "who cares, just don't be stupid" argument. Explicit declarations offer me four immediate benefits: * readonly as Nick demonstrates * separation of concerns inside the class * verbosity in constructor bodies * DOCUMENTATION! The "why should I care?" argument against readonly is as bad as saying "just make it public" - because you are creating the same opening for someone to exploit your code. As for field vs property - property gives me one huge benefit - setter control. Does nobody else use "private set" or "init"???
Honestly I prefer to use the primary constructor syntax and really it's not that hard to not mutate the injected dependencies. People built JS and Python huge projects without all this safety.
How about this for a terrible idea? Add an in -modifier to your primary constructor parameters. This makes them "readonly" and unusable outside the code generated constructor, making you unable to do anything but initialize fields and properties with it. E.g.: public class MyClass(in string parameter) { private readonly string _value = parameter; // This is fine public string Value { get; } = parameter; // This is also fine public void DoThing() { Console.WriteLine(parameter); // Compiler error CS9109 } }
I hope that they will add support for the readonly keyword in the custructor, e.g. like you can do it in TypeScript: constructor(private readonly Dependency dependency) {}
the easiest fix wuild be create ReadonlyAttribute and snap it to primiry constructor, it will look, well, not terable and doesn't introduce new behavior to the langueage
Can you make video about 1 brc challenge in c# ?
C++ has const parameters, it should be possible to mark a parameter as readonly in C#.
Kotlin has them (and everything else) as read only by default as well. It’s the better choice.
A developer dereferencing the value of the injected dependency to another value is very low probability. But in the worst case scenario, the behaviour only affects the current class as you are not affecting the original injected dependency (even if you set it to null or you dereference it) as you are not passing by ref. This could be easily detected by the developer writing the code, unit testing or code review. Feels that the provided solution is an overkill and might make adopting the language complicated for new developers.
While I agree that it is safer to use readonly than to have your variables be mutable, I don't think it's as big of an issue as people make it out to be. Even if your injected services are readonly, they are still mutable. The variables aren't, but you can very much change their state with a method call. In cases where variables are mutable, I don't recall ever seeing an issue of unintentionally assigning it, that wouldn't also be obvious in some other way. In contrast, unintentionally modifying the state of an object or service through a method invocation happens all the time, since it's much less visible.
I'll just use the old constructor. So much fuss over saving a few lines of code.
That's how I've been doing it, setting a readonly field, but I hate it. And definitely only use it when there's nothing else that needs to go in the constructor besides setting the fields; otherwise, what's the point?
IDEs just suggesting you can change this and even printing a warning is something I don't understand. I converted an entire project to primary constructors until I found out readonly wasn't possible. As they exist right now, primary constructors just don't make sense. They were pushed out too early IMO.
Good one.
I am fine with the new syntax. Also I don’t think missing readonly keyword is THAT critical. But what I really hate is that compiler forces me to use fields without underscore. Damn, how I suppose to know that some symbol is a field and is not a parameter or variable? Of course, I can use underscores in ctor parameters, but that way my parameters naming will be ugly for class consumers.
I really think think that primary constructors should maybe go away. I'm not loosing any sleep over typing out a few extra characters to be honest. I do not think that they offer or are able to offer the flexibility needed to be worth the work. And now I need to keep an eye for their usages in code review. So in the end, it's just more work for me. ugh.
More than 50% of fields in classes of my typical project are read-only fields, so I don't use Primary Constructors because of that specific reason
Also, what about the "not null" check? I think people have a lot of constructors with smth like "_dep = dep ?? throw new ArgumentNullException();"
Porting kotlin’s primary constructors didn’t go very well without its val and var keywords. The goal was to reduce boilerplate, so I highly assume the team has been debating trying to introduce val and var syntax into the language. It’s clear to me that with implicit typing in strongly typed languages emerging as king (see: rust), that `name: Type` wins out over `Type name`, and I can’t think of any usages of : in c# that would break backwards compatibility.
What about named parameters? They use colons.
@@vasiliychernov2123 I don’t think that conflicts because that’s not variable declaration. It’s only used in named tuples and when calling functions
I dont like primary constructors, its just less typing for the programmer with currectly limited feature set (no readonly). More typing is not a problem, but hiding stuff is a problem! Of course i dont want everything to be public, there is a reason for private, protected, internal. But why people always want to decrease the time how much typing you need... i dont understand. Typing code is not the hard or slow part! - Designing the architecture and the algorythmns is the slow part - Making the application fast and reliable is the slow part - Using the hardware efficiently you are given is the hard part And even if you really hate typing, then just let the IDE generate the code for you. Visual studio can do that, just put in the parameters you need and use intelli-J to create the readonly fields or properties automatically.
You can't just say: public record MyService(IDependent dependent) { public int extraModifiables { get; set; } } ?
class -> record fixed!
when you declare the private readonly field, use the same parameter name (don't prefix with _) - private readonly IDependent dependent = dependent; This makes it readonly without having to write a separate constructor so its still more succint.
Primary ctors are meh... I would not recommend any syntax to fix this. Just use an explicit readonly field if you need one. In that regard they probably should have made the (hidden) field readonly by default and so you'd use a custom field if you'd need a writable one... Oh, well - too late now.
For me, primary ctor is just a way to specify a kinda contract that all other ctors have to implement and that's it.
I think readonly with ctor signature would be good. I wouldn't use it in this case otherwise
08:45 There is no "compiler warning". I started using this pattern in my code everywhere and there are no warnings. The warning seem to be from Rider, since I don't get any complaints from Visual Studio. In fact, the "default" refactoring in VS does not remove your readonly fields when converting to a primary constructor. That also seems to be a Rider thing.
Visual Studio and VS Code give me two refactor options: one to convert to primary constructor which leaves the readonly field in place, and another to convert to primary constructor without the field. So I'll generally choose the one that leaves the field, unless something small enough that I don't care. Rider must have different default refactoring suggestions from VS - with this one, I like what I get with VS.
i believe you still can do the following define a field as readonly and assign the value from what comes from your constructor... public class MyService(IDependent dependent) { private readonly IDependent _dependent = dependent; public void Do() { _dependent.DoWork(); } } Granted, it's not as clean as not having to do that, but it should be fine if you must have a readonly
Nick already demonstrated that 'workaround' in this here video 😉
@Assgier Thanks man! I missed it lol. Edit: I see why I missed it! It was after he talked again about linkedin. I thought no more code would follow :D If anybody is curious: 8:26 is the mark
Also you can make any primary constructor parameter readonly using roslyn analyzer. No extra syntax needed.
This is why I hate all the sugar coated syntax in new C#. They are trying so hard to tailored this towards script kitty. No matter how they fix it, I will never use this feature. Use attribute to decorate primary constructor is best.
There's no null checks with this primary constructor too.
I just use it to remove the constructor, I still have my private readonly field public class SomeClass(IDependency dependency) { private readonly IDependency _dependency = dependency; }
I do the same but my field and parameter have the same name, so the field hides the parameter of the primary ctor
@@sineme Didn’t realise the compiler would let you do that, even better
And now your teammates who don’t understand will start using both the constructor parameter and the private fields within the same methods.
@@jkdmyrs Using the same name as suggested by the first comment would prevent this. Anyway I’m not worried about that, at some point you have to trust the people you work with and believe they’re not inept developers. Constantly being concerned with how overs may perceive or view code that is using standard language features is silly and will slow down your entire team
@@justgame5508Naive thinking I'm afraid. From my experience most developers sadly have the thought process of an infant. "If no red squiggly line then good". At that point I just would use the primary constructor without any additional code if you put "trust" in others anyway
I wish all properties were readonly and all variables were const by default. I think it’s mistake for an object oriented language to make properties mutable by default. It would be better if you had to explicitly mark them as mutable to make it known that some side effect updates the property.
i dont like primary constructor
I love Code Cop!! And yes, I care about the readonly...