Public readonly string vs. public readonly property?
I got an email asking me a question, and I thought it would make a good blog post. It has all the things that make a good Hollywood movie - Conflict, tradeoffs, and a grisly result if you choose wrong.
So, the question is, "Given the choice between a public readonly string or a public readonly property, which one is better and why?"
Just to make sure we're all on the same page, the public readonly string looks like this:
class ThingOne
{
public readonly string Value;
public ThingOne(string value) { this.Value = value; }
}
and the public readonly property looks like this:
class ThingTwo
{
private string value;
public ThingTwo(string value) { this.value = value; }
public string Value { get { return this.value; } }
}
So, what are the good points of each of these implementations?
The readonly one is simpler to read and understand and simpler to maintain (think of the differences in code with 10 of these in each class). It also has the advantage of being more precise, and by that I mean that the intent of the developer is crystal-clear; Value is something that is set when the instance is constructed, and never modified afterwards. That's not true with the property version - it could be modified by setting value in some other method, or adding a set accessor.
The property one provides the isolation that properties do - the implementation can be changed underneath without requiring the caller to change anything.
Those are the tradeoffs as I see them. Which one to use? Well, I think it depends on two factors:
1) How sure are you that this thing is truly readonly.
2) The pain if you're wrong.
Being less than convinced about my prescient abilities - and such abilities of developers in general - I think that #2 is the factor that you really need to consider. If the pain will be bad, you go with the properties approach. If it won't be bad, you go with the readonly approach. But, if you're really really sure that it's readonly and will always be readonly, then you can forget about the pain.
As for the pain potential, it depends mostly on what you're building and how your project is architected.
The first question is whether the class is visible outside of your assembly. If it isn't, then the pain of making a change is minimal, and I would prefer readonly because of the advantages I listed.
If it's visible outside the assembly, then the next question is "would we ever need to be update to update this assembly separately from its clients?" In other words, do you have servicing requirements?
My original question here was whether the class was used outside your group, but it really comes down to build and deployment scenarios. If your group is the sole user of the assembly or all referenced assemblies are shipped as part of an update, then I would continue to prefer readonly.
If, however, this class is an externally-visible class and you think that it might need to be serviced (even if you don't do that sort of thing now), then you need the isolation that properties provide.
My own thinking in this area has certainly evolved over time. The more code that I work with, the more I think that premature generalization ranks right up there with premature optimization in the seven deadly sins of programming. Code that is more complex than it needs to be takes longer to write, and you pay the complexity tax every time somebody needs to work with the code. The difference between a class with 10 public fields and one private/property and one with 11 properties, 10 of which are trivial is substantial.
You may have noticed that I didn't mention efficiency in this post until now. Because of the inlining of properties, there typically isn't any difference, but even if that wasn't the case, I wouldn't consider it a factor in my decision. I'm convinced that a group that writes clean and simple code, tracks perf, and optimizes as appropriate will end up with better perf than a group that tries to code performant code along the way. Simple code is quicker to write and easier to optimize later if necessary.
So, that's what I think. Agree? Disagree? Want to know what the other 5 deadly sins are?
Tom, thanks for the question.
Comments
Anonymous
March 17, 2006
Since developers have proven time and time again that they cannot responsibly use complex features of a language, I think it is time to go back to basics and take away their toys and give them basic tools that they cannot hang themselves with. If they want complex features, encapsulate it in a function.
"BACK TO BASICS" i say. Im tired of seeing the same mistakes we have made for the past 2 decades over and over again. Time for a redesign.Anonymous
March 17, 2006
Test for null, pfft IM too good for that.
Test for valid inputs, pfft im too cool for that.
Complex assignments on declarations: But it looks cool! and saves me seconds of typing!
SADAnonymous
March 17, 2006
STOP RAIDING IRELEVANT BUGS!! TESTING FOR NULL IS SO UNCOOL!!!
Man Im so envious of those SDE's. NOTAnonymous
March 17, 2006
MustCallThis(somecraphere).ThisWillAlwaysSucceed(YeahYeahBabyYeah)'
FRACK C#.
CRASH.Anonymous
March 17, 2006
I always lean towards flexibility and use properties wherever I can, so your original question is a no-brainer to me.
My trouble is with the use of readonly properties, a possibility you don't even mention here!
Ever since I discovered their existence I've been wondering what policy to use for applying them.
One approach is to never use them at all; after all, a readonly property can always be replaced with a function.
Another approach is to have no policy at all and use them ad libitum. This is OK except that arbitrarinessis bad in an API definition - the user should only have to learn relevant things, not arbitrary irrelevant decisions by the designer.
At present I am using readonly properties for members without side effects, and functions otherwise. But there is really no good motivaition for doing this.
Can you give me your own thoughts on whether/when to use read-only properties?Anonymous
March 17, 2006
The intent can be made clear in the property version too, by adding the readonly modifier to the private field.Anonymous
March 17, 2006
rp,
Can you explain what you mean by "readonly property"?Anonymous
March 17, 2006
What would be the drawback of rearchitecturing C# to make public variables compile to public get-only properties?Anonymous
March 17, 2006
I'd like to know the remaining seven deadly sins of programming, according to Mr. Gunnerson! :)Anonymous
March 17, 2006
There are also a number of frameworks out there that work using reflection and don't bother with fields. So if your class must be used by such a framework, you should use the property. Furthermore, won't code analysis complain about any kind of public field?Anonymous
March 17, 2006
EricGu (Microsoft) has made an excellant post on readonly strings and read-only properties. Check it...Anonymous
March 17, 2006
The comment has been removedAnonymous
March 17, 2006
I also want to know what the other 5 deadly sins are?:)
ShevaAnonymous
March 18, 2006
Nice read.. The other 5 sins would be nice to know.Anonymous
March 18, 2006
My comment got long so I posted my night-time thoughts on the dark side of properties on my web:
http://www.mashiharu.comAnonymous
March 18, 2006
The comment has been removedAnonymous
March 18, 2006
With a readonly property I mean a property with a get clause, without a set clause.
(You can tell I use VB.NET more than C#.)Anonymous
March 18, 2006
I should add, the first part of my comment doesn't make sense, since I misread some of your posting.
You are already using a readonly property!
So maybe I should just have written: I wouldn't soon replace a readonly string property with a readonly string,
since the property leaves some flexibility to the implementation, although it has drawbacks, e.g. a readonly property isn't actually guaranteed to not modify the value or to not have any other side effects;
but I do consider replacing any readonly string property with a string function. (I.e. omit the "get".)Anonymous
March 19, 2006
Oddly, the only exclusion to the Framework Design Guidelines' (Guidelines) "Do not provide instance fields that are public or protected" deal with constant and static read-only fields. Sadly, you provided no feedback or opinion on this section in Guidelines.
You appear to be contradicting the Guidelines by suggesting readonly instance fields are a better choice under certain circumstances. As is mentioned in the Guidelines (as other commenters have reiterated) publicly available interfaces (a "Framework") should be designed to evolve and encapsulate implementation. These concepts are repeated "mantraistically" in the Guidelines. Some of the basis for this guideline stem from realization that even some of the brightest people at Microsoft have designed interfaces optimized for maintainability of the resulting implementation, not for usability of the resulting API. And exemplifies that they have almost always been bitten for making this trade-off--despite these bright individuals putting much thought into their choice.
I would agree that, from the narrow view of optimized maintainability and implementation, readonly instance fields are quicker and can be viewed as easier to maintain in the short-term. But, I believe we've learned that providing a interface is about providing an interface first and foremost. I would be interested in hearing your views on whether these concepts from the Guidelines should still be taken into account when making decisions like this.Anonymous
March 21, 2006
Don't see a debate to be honest - surely it's the Property all the way :D Consistency, Maintainability, and miminal risk following change outweigh any other considerations here in my estimation.Anonymous
May 16, 2006
A while back, I made an offhand comment about something being one of the seven deadly sins of programmers...Anonymous
January 13, 2007
Seven Deadly Sins Of ProgrammingAnonymous
June 17, 2009
PingBack from http://pooltoysite.info/story.php?id=7551