8

When writing a class in C#, is it a good idea to mark all of you private member variables as private readonly if they are only assigned to in the constructor and aren't subject to change elsewhere in your class? Or is this overkill?

Bobbo
  • 1,593
  • 4
  • 13
  • 13

6 Answers6

11

Yes, personally I believe it's a good idea. I try to keep types immutable where possible, and declaring a variable readonly is a good start to that. It's not the be-all and end-all, of course - if that variable is something mutable (e.g. a StringBuilder or an array) then it's really not helping all that much. I'd still make the variable read-only though to make it obvious that I don't want to change the value of the variable itself - and to prevent myself from doing so accidentally elsewhere in the same class, possibly months or years later.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • you're a guru in both Java and C#. This is essentially the C# equivalent of this Java question. http://stackoverflow.com/questions/137868/using-final-modifier-whenever-applicable-in-java I wonder why the discussion here seems so much tamer than the Java one. – RAY Apr 24 '12 at 09:29
  • @RAY: No, bear in mind that `final` in Java applies to more than just variables - it's a combination of `readonly` and `sealed` in C#... and if you start asking folks whether classes should be sealed or not, you'll see more heated debate... – Jon Skeet Apr 24 '12 at 09:50
  • True. The linked the question specifically asks about local and class variables (arguments and fields) though... – RAY Apr 24 '12 at 10:18
4

Yes, that is what readonly specifically indicates. If you already know (or can at least assume) that you're not going to assign it anywhere else, then marking it readonly is a good idea. After all, it's easier to remove readonly than it is to add it later.

Adam Robinson
  • 182,639
  • 35
  • 285
  • 343
1

Wow what a good question and one that is purely going to be answered with opinions. My opinion is I always just create properties to the variable. An example is as follows.

private int _myInt;
private int myInt {get{return _myInt;}}
dko
  • 874
  • 2
  • 7
  • 18
  • 4
    That still allows the variable to be mutated within the class - it doesn't indicate the developer's intention *not* to mutate it within some other method. – Jon Skeet Jan 07 '11 at 16:14
0

Yes - you won't run into problems with their values being modified later on by some code written by other developer that didn't know that they should be read-only.

Jakub Konecki
  • 45,581
  • 7
  • 87
  • 126
0

Readonly makes very much sense in situations where you pass a service reference through constructor, i.e.

public class MyViewModel { private readonly MyContext context; public MyViewModel(MyContext context) { this.context = context; } }

You obviously don't want your context overwritten with another one, because you can have a lot of stuff dependent on that particular service inside the class. And if it's a constructor parameter, that usually means you RELY on that particular service or object for creating and keeping the valid state of the object. So readonly is a good indicator of just that. Having private set on property means that you can't change it outside the class, readonly is an extra constraint which makes things a bit more secure and understandable.

Bruno Altinet
  • 286
  • 2
  • 4
-1

If I'm only going to initialize a variable once and never write to it, I would make it const.

http://en.csharp-online.net/const,_static_and_readonly

Jonathan Wood
  • 65,341
  • 71
  • 269
  • 466
  • 1
    You can't make it const if the value won't be a compile-time constant, or if it's an instance field. – Jon Skeet Jan 07 '11 at 16:18
  • Right, I consider const more efficient because it *is* compile-time. If the member can't be const, then another route will be needed such as readonly. Otherwise, I'd make it const. – Jonathan Wood Jan 07 '11 at 16:26
  • 4
    Even if it is of the right type, it still might be a bad idea to make it a const. For example, suppose you have an int field "VersionNumber". Do *not* make that a const, make it readonly. A version number is a quantity which logically *changes over time*, and therefore is not *constant*. Only use const for things that *have never changed, and never will change*, like the value of pi or the atomic number of lead. – Eric Lippert Jan 07 '11 at 17:10
  • @Eric: That seems like an arbitrary rule. As long as the variable doesn't ever change while the program is running, a const seems like the ideal solution. – Jonathan Wood Jan 07 '11 at 17:16
  • 7
    It is not *in any way whatsoever* an arbitrary rule; it is a consequence of the fact that *constants are treated by the compiler as never changing*. Suppose you have assembly Alpha with constant field C.F set to 10. You compile assembly Beta which prints out C.F from Alpha. Now at runtime you replace Alpha with a difference Alpha.DLL where C.F is 20. **Beta continues to print 10**. The compiler has assumed that since you said *constant* you meant *I'm not going to change this*, because *that's what constant means*. If you want Beta to print 20 then C.F has to be readonly, not constant. – Eric Lippert Jan 07 '11 at 17:21