"The readonly Keyword Is Useless!" | Code Cop

  Рет қаралды 36,317

Nick Chapsas

Nick Chapsas

Күн бұрын

Пікірлер: 266
@Crozz22
@Crozz22 9 ай бұрын
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.
@swozzares
@swozzares 9 ай бұрын
Not if it has the same name
@martinprohn2433
@martinprohn2433 9 ай бұрын
​@@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.
@JoeEnos
@JoeEnos 9 ай бұрын
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.
@jongeduard
@jongeduard 9 ай бұрын
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.
@jell0goeswiggle
@jell0goeswiggle 8 ай бұрын
Oh wow, that's surprising. I had been preferring that last method but this definitely has me reconsidering.
@SuperJB1981
@SuperJB1981 9 ай бұрын
Thanks for the shout out @nick. Yours truely... JohnB :)
@orterves
@orterves 9 ай бұрын
6:43 remember - that "everyone" also includes yourself in six months. And that developer needs all the help they can get
@OhhCrapGuy
@OhhCrapGuy 9 ай бұрын
"six months"? Sometimes it's six weeks or six days.
@sineme
@sineme 9 ай бұрын
@@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
@sonicmouse3083
@sonicmouse3083 9 ай бұрын
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.
@bulgarian388
@bulgarian388 9 ай бұрын
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.
@StacyClouds
@StacyClouds 9 ай бұрын
I love this new syntax but do not understand why it wasn't implemented as a readonly field 😭
@jantajov
@jantajov 9 ай бұрын
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.
@SkyyySi
@SkyyySi 9 ай бұрын
​@@jantajovThat doesn't make any sense. By that argument, the readonly keyword should be removed from the language altogether
@jantajov
@jantajov 9 ай бұрын
@@SkyyySi can you tell me what do you think readonly keyword does? Don't google it, just tell me what you think it does.
@riten
@riten 9 ай бұрын
@@jantajov plugins are great, but we are talking of 'raw' C# here, native support.
@nothingisreal6345
@nothingisreal6345 9 ай бұрын
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.
@fusedqyou
@fusedqyou 9 ай бұрын
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.
@baturhankahraman426
@baturhankahraman426 9 ай бұрын
I assume we already have a keyword "in" for the functions. maybe they can add "in" to primary constructors instead of readonly.
@LeMustache
@LeMustache 9 ай бұрын
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...
@modernkennnern
@modernkennnern 9 ай бұрын
@@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.
@LeMustache
@LeMustache 9 ай бұрын
@@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.
@ThunderChasers
@ThunderChasers 9 ай бұрын
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.
@TehKarmalizer
@TehKarmalizer 9 ай бұрын
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.
@TicoCryptonaut
@TicoCryptonaut 9 ай бұрын
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.
@ThunderChasers
@ThunderChasers 9 ай бұрын
@@TicoCryptonaut exactly my thoughts!
@neverovnikita
@neverovnikita 9 ай бұрын
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
@neverovnikita
@neverovnikita 9 ай бұрын
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
@iantabron
@iantabron 9 ай бұрын
​@@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
@iantabron
@iantabron 9 ай бұрын
​@@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...
@neverovnikita
@neverovnikita 9 ай бұрын
​@@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
@iantabron
@iantabron 9 ай бұрын
@@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.
@Menicoification
@Menicoification 9 ай бұрын
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.
@TazG2000
@TazG2000 9 ай бұрын
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?
@Menicoification
@Menicoification 9 ай бұрын
@@TazG2000 Yes. It's not available becaus it' not used. If you'd use it in a method it would become available again.
@StuartQuinn
@StuartQuinn 9 ай бұрын
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?
@marcosroberto3576
@marcosroberto3576 9 ай бұрын
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.
@1Gargo
@1Gargo 9 ай бұрын
A removed readonly keyword is more likely to be noticed in a code review, though
@Crozz22
@Crozz22 9 ай бұрын
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".
@1Gargo
@1Gargo 9 ай бұрын
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.
@lolyasuo1235
@lolyasuo1235 9 ай бұрын
Nobody will remove a readonly keyword. They will simply don't put it on newer implementations.
@iantabron
@iantabron 9 ай бұрын
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"???
@Stoney_Eagle
@Stoney_Eagle 9 ай бұрын
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.
@jongeduard
@jongeduard 9 ай бұрын
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.
@Stoney_Eagle
@Stoney_Eagle 9 ай бұрын
@@jongeduard Thanks for the clarification, that makes a lot of sense!
@jongeduard
@jongeduard 9 ай бұрын
@@Stoney_EagleYou're welcome.
@JustArion
@JustArion 9 ай бұрын
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)
@sonicmouse3083
@sonicmouse3083 9 ай бұрын
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.
@NickSteffen
@NickSteffen 9 ай бұрын
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.
@alonmore28
@alonmore28 9 ай бұрын
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.
@Matlauk
@Matlauk 9 ай бұрын
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.
@1Gargo
@1Gargo 9 ай бұрын
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.
@lolyasuo1235
@lolyasuo1235 9 ай бұрын
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.
@stephajn
@stephajn 9 ай бұрын
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.
@KodingKuma
@KodingKuma 9 ай бұрын
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.
@dune102
@dune102 9 ай бұрын
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 🙂
@jongeduard
@jongeduard 9 ай бұрын
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.
@computer9764
@computer9764 9 ай бұрын
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?
@sam2943
@sam2943 9 ай бұрын
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.
@ilyakurmaz
@ilyakurmaz 9 ай бұрын
I am using this feature with Mediator freely without any issues, so not sure about the point of handlers.
@LE8271
@LE8271 9 ай бұрын
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.
@jamienordmeyer4345
@jamienordmeyer4345 9 ай бұрын
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.
@muemmel20
@muemmel20 9 ай бұрын
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.
@vyrp
@vyrp 9 ай бұрын
"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.
@JoeEnos
@JoeEnos 9 ай бұрын
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.
@muemmel20
@muemmel20 9 ай бұрын
@@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.
@muemmel20
@muemmel20 9 ай бұрын
@@vyrp Fair enough. That would mostly apply to NuGet Packages and non-monolithic solutions though.
@muemmel20
@muemmel20 9 ай бұрын
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.
@RealCheesyBread
@RealCheesyBread 9 ай бұрын
According to the principal of least privilege, the readonly keyword is needed for every field that doesn't need to be modified (or shouldn't be modified).
@vadimk4310
@vadimk4310 9 ай бұрын
How to validate parameters of primary constructor 🤨?
@asherrfacee
@asherrfacee 9 ай бұрын
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)); }
@daniilpalii6743
@daniilpalii6743 7 ай бұрын
If you need constructor with body - just use constructor with body
@seangwright
@seangwright 9 ай бұрын
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.
@blazjerebic8097
@blazjerebic8097 9 ай бұрын
knew what you were about to say as soon as I saw "readonly" on a Code Cop video notification
@justinharris6197
@justinharris6197 9 ай бұрын
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.
@lolyasuo1235
@lolyasuo1235 9 ай бұрын
Primary constructors are not useful in any way. And the reason is readability. It is ugly and harder to review: public class MyService( IServiceA servicea, IServiceB serviceb, IServiceC servicec, IServiceD serviced, IServiceE servicee, IServiceF servicef, ) { public void Do() { } } PS: How many of you seeing this code ^ think about missing class brackets? Well. Nothing is missing in the above code and will compile just fine :). You just confused.
@mdcorbett
@mdcorbett 9 ай бұрын
It really doesn't fit well with the language's overall "look," in my opinion.
@qwer1234cvb
@qwer1234cvb 9 ай бұрын
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%).
@ThomasAngeland
@ThomasAngeland 9 ай бұрын
I'll just use the old constructor. So much fuss over saving a few lines of code.
@chlorobyte_projects
@chlorobyte_projects 9 ай бұрын
C++ has const parameters, it should be possible to mark a parameter as readonly in C#.
@desertfish74
@desertfish74 9 ай бұрын
Kotlin has them (and everything else) as read only by default as well. It’s the better choice.
@TheTigerus
@TheTigerus 9 ай бұрын
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.
@coloresfelices7
@coloresfelices7 9 ай бұрын
You can easily do that.
@MZZenyl
@MZZenyl 9 ай бұрын
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.
@ryan-heath
@ryan-heath 9 ай бұрын
By this reasoning you can’t ever use new syntax, because other parts of your code base is not using it 😅
@MZZenyl
@MZZenyl 9 ай бұрын
@@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.
@ryan-heath
@ryan-heath 9 ай бұрын
@@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.
@MZZenyl
@MZZenyl 9 ай бұрын
@@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.
@ryan-heath
@ryan-heath 9 ай бұрын
@@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 ...
@Kommentierer
@Kommentierer 9 ай бұрын
Feels a bit like an oversight from the design process of primary constructors. Why not allowing to specify access modifiers on it?
@JefferyBell
@JefferyBell 9 ай бұрын
7:34 “it’s amazing, it’s great!” 😂
@_xentropy
@_xentropy 9 ай бұрын
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.
@99aabbccddeeff
@99aabbccddeeff 9 ай бұрын
Thank you for these cop series. They are amazing!
@DanimoDev
@DanimoDev 9 ай бұрын
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.
@20windfisch11
@20windfisch11 9 ай бұрын
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.
@danielfpv86
@danielfpv86 9 ай бұрын
Who cares? How often has "somebody set the DI service member to null" actually be a problem within a dev team?
@lolyasuo1235
@lolyasuo1235 9 ай бұрын
I think you're joking, right? are you new to developing?
@tanglesites
@tanglesites 9 ай бұрын
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.
@ckopf86
@ckopf86 9 ай бұрын
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.
9 ай бұрын
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
@JoeEnos
@JoeEnos 9 ай бұрын
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.
@elliotdematteis7825
@elliotdematteis7825 9 ай бұрын
I don't see any reason to move to primary constructor when it's a straight downgrade to save 2 lines...
@swozzares
@swozzares 9 ай бұрын
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.
@andersborum9267
@andersborum9267 9 ай бұрын
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).
@lordmetzgermeister
@lordmetzgermeister 9 ай бұрын
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.
@jantajov
@jantajov 9 ай бұрын
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.
@Gattuser
@Gattuser 9 ай бұрын
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.
@jantajov
@jantajov 9 ай бұрын
@@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.
@Gattuser
@Gattuser 9 ай бұрын
@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
@jantajov
@jantajov 9 ай бұрын
@@Gattuser There isn't. Primary constructors cannot have read only parameters.
@tauheedulali2652
@tauheedulali2652 9 ай бұрын
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.
@tauheedulali2652
@tauheedulali2652 9 ай бұрын
This is my example: private readonly int _dependent = dependent; private int Dependent => _dependent;
@rankarat
@rankarat 9 ай бұрын
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.
@diligencehumility6971
@diligencehumility6971 9 ай бұрын
I honestly think primary constructors are an error in dotnet. Only usefull in simple classes. IDE is constantly screaming at you with errors "use primary constructor", the you do it, but thwn you have to re-write it latter because you need some init logic. There should be only one way to construct a class, not two
@martinprohn2433
@martinprohn2433 9 ай бұрын
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.
@isnotnull
@isnotnull 9 ай бұрын
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
@DisturbedNeo
@DisturbedNeo 9 ай бұрын
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.
@Dimich1993
@Dimich1993 9 ай бұрын
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.
@diadetediotedio6918
@diadetediotedio6918 9 ай бұрын
6:50 Let's be real, the reason we don't just use fields is just because C# is entangled with OOP, but apart from the code standardization and the potential (never will be) refactoring to use the get; set; of the property, it is just basic boilerplate and pollution. But I agree with the feeling on readonly fields, I generally use primary constructors this way: public class X(string name) { private readonly string _name = name; } It works as well and don't generate the normal field, and it is still nicer to work with
@Quxer0721
@Quxer0721 9 ай бұрын
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.
@DevonLehmanJ
@DevonLehmanJ 9 ай бұрын
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(); } }
@obiwanjacobi
@obiwanjacobi 9 ай бұрын
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.
@phizc
@phizc 9 ай бұрын
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.
@slowjocrow6451
@slowjocrow6451 9 ай бұрын
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"
@RandallEike
@RandallEike 9 ай бұрын
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.
@ErazerPT
@ErazerPT 9 ай бұрын
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".
@ryanzwe
@ryanzwe 9 ай бұрын
Wow, ive just setup a new service and used primary constructors everywhere..none of us knew this 😂.. time to go back!
@PedroPabloCalvoMorcillo
@PedroPabloCalvoMorcillo 9 ай бұрын
Don't do that. It's fine.
@modernkennnern
@modernkennnern 9 ай бұрын
This is a non-issue. Just install an analyzer instead it if you're worried.
@ryan-heath
@ryan-heath 9 ай бұрын
dont rollback, this is really a non-issue dispite the backlash ...
@StuartQuinn
@StuartQuinn 9 ай бұрын
Did anyone reassign a dependency? I bet they didn't.
@ryanzwe
@ryanzwe 9 ай бұрын
After sleeping on it, it sounds pretty silly for me to change it all back
@proosee
@proosee 9 ай бұрын
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.
@dlhsnbrn1275
@dlhsnbrn1275 9 ай бұрын
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.
@JoeEnos
@JoeEnos 9 ай бұрын
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.
@torrvic1156
@torrvic1156 9 ай бұрын
Thank you so much for your awesome tutorials Nick!
@jholman-thrive
@jholman-thrive 9 ай бұрын
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.
@daniilpalii6743
@daniilpalii6743 7 ай бұрын
Injecting DbContext with primary constructor works for me
@fifty-plus
@fifty-plus 9 ай бұрын
Primary ctors with an analyzer works nicely.
@marcusmajarra
@marcusmajarra 9 ай бұрын
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.
@mumk
@mumk 9 ай бұрын
i dont like primary constructor
@xeuszzz
@xeuszzz 9 ай бұрын
For the IDE to sidestep the use of readonly keyword and suggest the "simplification" is absolutely asinine. WTH, Rider? 😱🤬
@denielalain5701
@denielalain5701 9 ай бұрын
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
@petrucervac8726
@petrucervac8726 9 ай бұрын
Although it would be great to have parameters as readonly, it's not a big deal if they aren't. The impact on the codebase will be minimal. I personally don't use them as I don't vibe with the syntax
@jantajov
@jantajov 9 ай бұрын
I 100% agree. If you care about having them readonly just add roslyn analyzer. No need for language support.
@bruno-id1wh
@bruno-id1wh 9 ай бұрын
The example code in the LI post wouldn't compile anyway as there's a typo. The readonly property is of type IDepedent 🙃
@mrgrovesempire
@mrgrovesempire 9 ай бұрын
Who cares. Just be disciplined enough not to change the variable. The example of a property being the same concept is not the same and misleading. Properties are for consumers. You don't have control of them, but constructor variables are all internal, and you have complete control. I feel this is just another thing to argue over, and it makes no difference. If you have the urge to change a variable from the constructor (which will be something that you wouldn't think to change in that code), you should probably go back to CS 101 and refresh on the basics.
@Bliss467
@Bliss467 9 ай бұрын
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.
@vasiliychernov2123
@vasiliychernov2123 9 ай бұрын
What about named parameters? They use colons.
@Bliss467
@Bliss467 9 ай бұрын
@@vasiliychernov2123 I don’t think that conflicts because that’s not variable declaration. It’s only used in named tuples and when calling functions
@TheKuj4
@TheKuj4 9 ай бұрын
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.
@mirrorless_mirror
@mirrorless_mirror 9 ай бұрын
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.
@Synesthesia-r9
@Synesthesia-r9 9 ай бұрын
I would still prefer immutability by default; whether that's a `val` or `mut` keyword, I don't care; it's better than being open when forgetting to set it as `readonly`
@mciejgda88
@mciejgda88 9 ай бұрын
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
@AdamTibi
@AdamTibi 9 ай бұрын
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.
@peterfriberg1960
@peterfriberg1960 6 ай бұрын
Do you have to sell domtrain evry minute?
@hektonian
@hektonian 9 ай бұрын
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 } }
@oliverpk
@oliverpk 9 ай бұрын
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) {}
@yuGesreveR
@yuGesreveR 9 ай бұрын
Absence of readonly just makes primary constructors useless, in my opinion. Really, how often do we need mutable fields injected from the constructor? It is a quite rare case, I think. The right thing was to make auto-property readonly by default. Right now they have just wasted a good idea without any reason
@mdcorbett
@mdcorbett 9 ай бұрын
Yeah, I think he has the uselessness backward here. Plus, not everyone is working with newer .NET projects, supporting primary constructors.
@CryShana
@CryShana 9 ай бұрын
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
@brianm1864
@brianm1864 9 ай бұрын
I love Code Cop!! And yes, I care about the readonly...
@buriedstpatrick2294
@buriedstpatrick2294 9 ай бұрын
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.
@TedFanat
@TedFanat 9 ай бұрын
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
@mrexvel
@mrexvel 9 ай бұрын
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.
@nage7447
@nage7447 9 ай бұрын
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
@andrewmaksymiuk986
@andrewmaksymiuk986 6 ай бұрын
Why don't they just do it with 'in' constructor parameters? Is there any reason why an 'in' parameter can't be assumed to be made into a readonly field?
9 ай бұрын
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...
@pagorbunov
@pagorbunov 9 ай бұрын
For me, primary ctor is just a way to specify a kinda contract that all other ctors have to implement and that's it.
"Stop Using Properties in C#, Just Use Fields" | Code Cop #013
11:53
Properties were getting even cleaner in C# 11
8:12
Nick Chapsas
Рет қаралды 57 М.
Chain Game Strong ⛓️
00:21
Anwar Jibawi
Рет қаралды 41 МЛН
Junior Developer Sent Me A PR For Review
17:26
Amigoscode
Рет қаралды 170 М.
"Stop Using null, Use default Instead in C#" | Code Cop #010
7:06
Nick Chapsas
Рет қаралды 67 М.
Await Async Tasks Are Getting Awesome in .NET 9!
9:24
Nick Chapsas
Рет қаралды 102 М.
How Senior Programmers ACTUALLY Write Code
13:37
Thriving Technologist
Рет қаралды 1,6 МЛН
"Stop Using Async Await in .NET to Save Threads" | Code Cop #018
14:05
"Don't Use Loops, They Are Slow! Do This Instead" | Code Cop #011
9:51
Reacting to Controversial Opinions of Software Engineers
9:18
Fireship
Рет қаралды 2,1 МЛН
this is what happens when you let the intern write code.
12:50
Low Level
Рет қаралды 385 М.
Make Your LINQ Up to 10x Faster!
11:08
Nick Chapsas
Рет қаралды 57 М.
Introducing Clay - High Performance UI Layout in C
35:19
Nic Barker
Рет қаралды 39 М.