One of the questions I’m asked frequently regarding design of C# classes is “should this be a property or a method?” Here are my guidelines:
- First and foremost, the thing in question should logically be a property of the thing being modeled by the class. The colour of a crayon, the mass of a car, the birthdate of a customer, and so on, are all logically properties. In particular, think carefully before making static properties; there are not many things which are logically properties of an entire class of things.
- Properties should be stable; that is, the value of a property shouldn’t change “on its own”. Rather, changes in properties are the results of calling property setters, or some other action changing the state of the object. The mass of a car shouldn’t change on its own, but if you drive it for a while, the fifty kilograms contributed by the mass of the gasoline might go down.
I note that
DateTime.Nowviolates these first two guidelines; the current time is not really a property of the set ofDateTimes, and it changes frequently on its own. In retrospect, it ought to have been aDateTime.GetCurrentDateTime()method. - Properties should be cheap; reading or writing the value of a property should be not much more than, say, ten times the cost of doing the same to a field. I note that if the jitter is optimizing your code, properties which are simple wrappers around field accesses will likely be inlined. That is, the code is generated as though the accessor usage was replaced with the accessor body itself.
- A common pattern is for a property to be a cache of an expensive, lazily-computed value; Roslyn uses this pattern a lot. Though the property is expensive on its first access, every subsequent access is cheap, so the property is cheap in an amortized sense. Be very, very careful with this pattern, particularly if you also want the property to be threadsafe. The number of ways to get it wrong is large. Consider using the
Lazytype as an implementation detail rather than trying to get all the details right yourself. - Property getters should not change the state of the object, except perhaps to cache state as mentioned in the previous point.
- Property getters should always succeed; they should never throw an exception. Return some reasonable default value if the property cannot be logically computed right now because the object is in a bad state. Similarly, allow setters to be called in any order; objects where setters can only be called when the object is in a particular state are error-prone.
- When you examine the state of an object in the debugger, by default the debugger calls the getters of all the properties of the object. (You can turn this behaviour off in the debugger options.) This means that if you have violated the previous guidelines, debugging a program can cause threading issues, change the states of your objects, or throw exceptions (which the debugger will shrug off, but still, it’s irritating.) This can make debugging programs a lot harder.
- As I mentioned a long time ago, try to not return arrays (or equivalently,
Lists) from properties. Arrays are expensive to create, and since the user can arbitrarily change the contents of the array, you can’t cache the array for later; you’ve got to recompute it every time. Use theReadOnlyCollectionwrapper class if the consumer of the property is not intended to be able to modify the data returned. - Mutable values types are of course pure evil and should be avoided; yet another reason for this judgment is: calling a method which mutates a value returned from a property getter mutates the copy that was returned rather than changing the copy stored in the object.
All that said, sometimes there will be conflicting guidelines. For example, should a property getter of a disposable object throw an exception if accessed after the object is disposed? We have one guideline that says that disposable objects should throw after they’re disposed and another which says that property getters should never throw, so which is it? I’d be inclined to return an invalid value rather than throwing, but I can see the argument the other way. I’ve had situations where I had the choice between a static method, a public readonly static field and a static property that isn’t really a “property” of the class; it’s not necessarily clear which guidelines are the most important. So take this advice in the spirit of guidelines, not rules.
Fabulous adventures in coding 
Nice post.
“Property getters should always succeed; they should never throw an exception.”
There are a few places in the BCL where this principle is violated, and it annoys me to no end. One of my favorites:
> var cts = new CancellationTokenSource();
> cts.Dispose();
> cts.Token.CanBeCanceled
ObjectDisposedException: The CancellationTokenSource has been disposed.
There is, of course, no property that can be used to evaluate whether a CancellationTokenSource has been disposed. At least the design enforces careful management of object scope and lifetime.
“by default the debugger calls the getters of all the properties of the object” — This is really useful information. Thank you!!
Other than that, though, most of these items feel more like coding-style recommendations than logical results of functional differences between properties and methods.
Countless times I’ve been bitten by libraries with non-idempotent getter properties, getters which return tricksy mutable copies, and setter properties with disproportionate side-effects. Are there any other important functional differences that we’d lose by just avoiding them? As a maintainer, I’ve never observed that they’ve improved code quality.
There are certainly developers who would rather do away with properties entirely (*cough* Jeffrey Richter), but if you adhere to reasonable guidelines, I think the improvement to code readability is sufficient to justify their existence. I find it useful to have a separate syntax (and separate syntax highlighting rules in my IDE) for properties, which describe qualities or state of an object, and methods, which perform some action on an object. There are other niceties like being able to use compound operators like `+=` with properties (more convoluted with setters). You also get less pollution of IntelliSense completion lists for read/write properties, as a single entry covers both accessors, and you get more sensible sorting than with “Get/Set” method prefixes.
If you’re being bitten by poorly designed properties with unexpected side effects, I’m not sure there’s any reason to assume that the same developer would have written better, more maintainable code using only methods.
In my experience, any statement that the compiler accepts, no matter how boneheaded or misguided, will (given enough time and collaborators) be checked into the project.
I kid you not, the other day I saw the following existential comment in a checkin:
switch(someBooleanExpression) {
case true: …
case false: …
default: // how can this happen??
}
No doubt. But if a developer decides to ignore the guidelines and implement properties with crazy side effects, then why would you expect the same developer to suddenly write better code just because he has to rely solely on methods?
Granted, there are some assumptions that a caller makes about properties, i.e., that a read access has little overhead and no unintended side effects. One might posit that we wouldn’t rely so heavily on those assumptions if we only had methods. But my time is split pretty evenly between C# and Java development, and my expectations for simple value accessors/modifiers are pretty consistent regardless of whether properties are available. Moreover, I still come across horribly counter-intuitive APIs where methods that ought to be pure have crazy side effects.
No matter what language features are available, a lousy developer is going to write lousy code. Some language features might encourage bad behavior more than others. Some developers might say that properties are such a feature, but I am largely unconvinced.
I agree. Java has no properties, but has conventions for getter and setter methods that mean that the exact same concept is there. Eric’s guidelines are just as good style for Java getters and setters as they are for C# properties.
In other words, the person who writes bad properties would simply be writing bad getters and setters in Java, or a hypothetical propertyless version of C#.
Although I will agree it is very likely so, this isn’t necessarily boneheaded or misguided. One possibility is that the developer preferred a switch over an if statement for readability (unusual but I can sometimes understand that), returns a value in the true case, and in the false case, but then received a compiler error “not all code paths return a value”. Another possibility is when you’re dealing with programs where your code is trusted, code of others is not, some of your code is callable by untrusted code, and that code takes boolean parameters: the untrusted code may be passing in values that are neither true nor false, such as (invalid C#) (bool)2, in order to exploit your bad security. A three-way switch can be used to handle that.
Using a switch with a default as a means of testing for an invalid Boolean variable might be a good idea (especially if a compiler would be allowed to optimize `someBool == true` to `(int)someBool != 0`), but a comment like `// Some (corrupt) non-zero value other than a valid true` might be more helpful.
I believe that properties are really, really necessary when the object(s) of your class(es) are going to be examined and accessed by something other than code: something like an IDE (good old Delphi), or any kind of discovery protocol (COM/DCOM/CORBA/WSDL/WCF and what not). Another example: a commercial class/component library (not an open source one), intended to be used by different environments and/or languages.
If your stuff is accessed only from code written in the same language and only by your trusted (I emphasise: trusted!) colleagues, then public members will do just as well–and will be more efficient, even if by a few nanoseconds. Unless, of course, we’re perfectionists and concerned about the coding style. 🙂
Why emphasize “trusted”? What is it about properties that is safer to expose to your colleagues than properties?
Just kidding: if you don’t trust your colleagues, changing jobs is the way to go. 🙂
If your stuff is accessed only from code written in the same language and only by your trusted (I emphasise: trusted!) colleagues, then public members will do just as well–and will be more efficient, even if by a few nanoseconds.
I disagree with this part, although I think the rest is quite true. Most compilers these days will inline simple getters and setters, so there will be zero difference in performance between a property and a public member variable.
But one screw-up respecting your public member variable will cost hours of dev time, retest time and, in the worst case, all the hassle of distributing a new build to the customer. All this to save only a few nanoseconds (if the compiler didn’t inline it anyway)? Your public member would need to be accessed a billion times a day for this to be worthwhile.
You may trust your colleagues, but I don’t trust myself not to screw up that much.
That’s definitely one point in favour of writing properties: if at some later stage you need to add more bells and whistles to the value you’re exposing, then in the case of a property, you just modify your getter/setter, and no one’s the wiser; in the case of a public member, however, you’re going to have to replace it with a method (a breaking change in the interface, with all that implies).
Fields can be passed as `ref` or `out` parameters, and fields of aggregate types can be efficiently modified in place. Neither such characteristic applies to properties.
For most class types, it is possible to imagine situations where there could be advantages to replacing fields with properties. Among other things, because class objects have *identity*, using properties makes it possible to have code to something special when a particular property of a particular instance is read or written.
For structure types which are intended to stick a bunch of values together with duct tape (e.g. a `Point`), exposed fields are often better. If a type’s semantics promise that all state may be read using public properties, and promise to allow the creation of instances whose properties have any combination of values that would be legal for their respective types, the only difference between using fields and properties will be that using fields will allow some operations to be done more efficiently than using properties. Anything one could do to a struct with exposed fields could be done to a struct which satisfied the promises indicated above *except* that some operations which would be thread-safe if done on fields could not be made thread-safe when using properties.
“A common pattern is for a property to be a cache of an expensive, lazily-computed value”
Although I do see the value in this, I am inclined to avoid it when possible. In the system I currently work on, it is heavily employed (nearly on every property), and disguises database calls. In many cases, developers forget to memoize the result, so accessing the getter multiple times gets you a different object representing the same db record and can generate an exceptionally large number of database calls. Perhaps there is a difference between having an expensive CPU bound operation and an external (db/filesystem/whatever) call hidden behind the getter. If the computation were idempotent (as I assume is the case for Roslyn), it would change my thinking.
It also causes problems with the debugger. The other day, I was debugging a property that looked like this:
private Foo mFoo;
public Foo TheFoo
{
get
{
if(mFoo == null)
mFoo = Foo.Read(TheFooId);
return mFoo;
}
}
I was stopped on the line that assigns mFoo, and unwittingly moved my mouse over the property declaration. The debugger evaluated the property, which assigned a value to mFoo, and then I was left wondering how I was on the assignment line when mFoo was non-null. I had to call a co-worker over to check my sanity. Fortunately, we figured it out in a relatively short period of time.
Lazy/computed properties are probably the most compelling argument against properties as a language feature. I don’t see that kind of abuse often enough to think we’d be better off without properties, but there should be much stronger guidance about when to use computed properties. That guidance should (in my opinion) place a heavy emphasis on avoiding them, and also provide suggestions for alternatives.
If a property `Foo` requires going out to the database, then it really should not be a property. Likewise, it shouldn’t be a `GetFoo()` method, because that doesn’t give the caller any reason to suspect there might be additional overhead. In these cases, I am strongly in favor of methods like `LoadFoo()` or `FetchFoo()`. The names clearly imply that I/O may be involved, and it encourages the caller to think, “well, if I have to load the value, I should probably reuse the result when appropriate”.
I have a theory that most abuses of properties can be traced back to the data binding capabilities of UI frameworks being so heavily dependent properties as binding sources. Perhaps if WPF (and, subsequently, Silverlight and WinRT) had been designed with a less property-centric approach to data binding, these abuses would be less rampant.
I see a place for computed properties where the computation is cheap – not the “cache something expensive and reuse the result” pattern you are talking about.
If I want to expose a piece of data that can be represented in multiple forms – say, a physical quantity that has one unit in the database and another on screen, but where having a “units” type hierarchy would be over-engineering – I’ll write multiple properties to expose these multiple forms. It’s just a convenience for users of my class.
In one case I remember, the codebase made extensive use of hierarchical names for pieces of data – basically paths, but not on a filesystem. Sometimes these paths were strings delimited by ‘/’, sometimes they were objects that maintained a list of string elements internally. There were handy functions that converted one to the other.
It turns out that when we profiled, those handy functions took about 10% of the total CPU used by the process. It was big-iron server programming, so 10% was a big deal. As paths were passed around the codebase, they were converted from one form to the other over and over and over again. Different developers had different preferences, and there was no pattern or convention.
In that case, presenting a path used by your class in both forms, to let the caller use what they like, would be one way to both optimise and simplify the code. Internally, use an object, and convert it to a string when someone wanted a string. That way, you guarantee that the cheaper of the conversion methods was used, and you let them write simpler code.
Another way would have been to settle on one representation for everyone, but it was a large codebase and incremental change was kind of necessary.
“I see a place for computed properties where the computation is cheap”
Agreed. I did not meant to imply that *all* computed properties should be avoided–just the ones that are deceptively expensive (particularly if they involve I/O).
You are just scratching the surface here, there are many more problems with this approach.
To start with, it violates the guideline that property getters should not throw. If you’re doing I/O, you can throw.
Secondly, because it’s doing I/O, the value of the property may differ depending on when you read it (assuming the underlying database isn’t read-only). This can cause all kinds of race conditions.
Then again, this is exactly what is done by most ORM’s, including LINQ to SQL and the Entity Framework.
“You are just scratching the surface here, there are many more problems with this approach.”
The comment box is only so big 😉
As a user of Entity Framework, I consider this a cost that is far outweighed by its benefit.
For the most part, EF doesn’t use I/O behind properties; it sets properties as a result of a read operation, not the other way around. The case where it differs is when the user explicitly marks a property to be a “navigation property”. That way you can call Person.Address.City without having to know ahead of time that you’re going to be loading in the address table. Of course, by default, Person.Address would only load when included in the query or when a separate one is made. The bad behavior is optional.
“Property getters should always succeed; they should never throw an exception. Return some reasonable default value if the property cannot be logically computed right now ”
Wouldn’t you say that calling a property at a time when it shouldn’t be called is a programming bug (clear contract violation) that should be brought as quickly as possible to the programmer’s attention? A default value can easily be overlooked and hide a bug. On the other hand the debugging behavior in VS makes throwing exceptions annoying.
Personally I find anything complex enough to possibly throw an exception depending on the state of the object shouldn’t be a property to begin with, that avoids this problem. Is that unreasonable?
“Personally I find anything complex enough to possibly throw an exception depending on the state of the object shouldn’t be a property to begin with, that avoids this problem. Is that unreasonable?”
I would agree that it’s preferable to strive for a design in which the properties of a disposable object can always return a meaningful value, regardless of whether the object has been disposed. This will not always be practical, but there are types in the BCL whose designers could have tried harder:
> var cts = new CancellationTokenSource();
> var token = cts.Token;
> cts.Dispose();
> token.CanBeCanceled
True // oh, really?
> cts.Cancel()
ObjectDisposedException: The CancellationTokenSource has been disposed. // oops, guess not…
“Property getters should always succeed; they should never throw an exception.” – There are so many cases in the .NET Framework itself where this is violated.
Agreed. Have you ever used the Exchange Web Services Managed API? Pretty much every property getter can and will throw an exception if you don’t initialise and/or call a function first to ‘pre-load’ the properties you require. I find it a real pain to work with!
Reading this I remembered of Form.Size and other properties like that… This part of the library wasn’t very well designed.
Much of WinForms design is incoherent, likely because many different aspects were all evolving at once. One of the worst parts is the `Control.Font` property. Nothing I can find documents who if anyone is responsible for disposing a font which is assigned to a control, and controls seem bizarrely oblivious to the disposal of their `Font`. If one sets a control’s `Font` property to an already-disposed `Font` object, the built-in control rendering methods won’t care that the `Font` was disposed, but reading the `Font` property will yield a reference to the disposed font, and passing that to `DrawString` will throw an exception.
I notice you make very few restrictions (suggestions) for property setters altering the state of the object or throwing exceptions.
Offhand I can only think of a few situations where you do not want to have other classes relying on the set value, such as connection details or SecureStrings. Would setter only properties be better if presented as methods?
Using the thought process behind these guidelines under what situation would a write-only property be justified over using a method?
“Would setter only properties be better if presented as methods?”
If the purpose of a property is to expose some quality or state of an object, then I question the usefulness of a property that doesn’t fulfill that purpose. Why declare a property for a value that cannot be inspected? You’re basically telling the consumer of the API, “Hey, just so you know, this object has a property ‘X’. You’ll never be able to know what ‘X’ is, but I feel it’s important for you to know that ‘X’ exists, and that some decisions may be made based on the value of ‘X’. I’ll even let you tell me what you think the value of ‘X’ *should* be, but I won’t necessarily tell you if I accept your value, or if it gets overwritten.”
I think that when most of us see a property, we assume we should be able to read its value, and (speaking for myself) we may experience brief confusion when the compiler tells us, “no, you can’t do that.”
So, yes, I would argue that write-only qualities are best set via methods. I’m interested in hearing Eric’s take, but I suspect he agrees (feel free to prove me wrong, Eric!).
A write-only property in isolation wouldn’t generally be useful, but some cases it may make sense to have a contravarariant interface expose a write-only property (especially an indexed one), have covariant interface expose a corresponding read-only property, and have an invariant interface which inherits those expose a corresponding read-write property. That pattern would be nicer if the read-only and write-only properties could be used together as though they were a read-write property, but for some reason if an interface implementing a read-only property and and another implementing a write-only property are both inherited at the same level, compilers seem unable to use the fact that a property is being *READ* or *WRITTEN* in deciding whether to use the *READ*-only or *WRITE*-only property.
About properties throwing on a disposed object: I feel that if “the thing in question is logically a property of the thing being modeled by the class”, than it will remain so after the object has been disposed. After all, properties should be stable.
For example, I see no reason why the ConnectionString property getter on a SqlConnection object should throw, just because you’ve closed the connection.
Consequently, we don’t have to check for the object being disposed in the property getter, which means the getter can remain cheap.
Eirc, for the second point, I’m not convinced.
I share the view that a property of an object is used to represent the *logical* property of the thing the object is modeling, and I also think this is the foundamental rule. However, when it comes to the stableness of a property, I don’t think it’s a necessary \property for a property to have. Why can’t a Person’s Temperature change on its own? And why can’t the Time change over time?
Another problem is that, how can you divide between ‘on its own’ and ‘by code’? The line may be ‘volatile’/’not volatile’ when we use properties to directly interface I/O, but that’s not always the line matters. The line matters is observer/observee. When you construct a Car. and throw it into a physical simulation loop, the Velocity property of the Car changes ‘on its own’ in your observation, but for the code implementing the loop, the property is changed ‘by the code’.
I think it comes down to your definition of “on its own”. If yours coincides with Eric’s, you will agree with him, if not, then maybe not.
I initially read it as meaning “a getter’s return value should only change if someone calls the setter”. Wait, what? But reading further, I understood this was not what he meant.
Eric said:
changes in properties are the results of calling property setters, or some other action changing the state of the object
I guess Eric meant, in other words, user should be safe to assume that the property of the object does not change, if no methods(including setters) are invoked on the object.
Is that what you read?
Well, yeah, the sentences on either side were also important in seeing how I arrived at my initial reading and my final reading. My reading of it started to change half way through the sentence you quoted.
My final reading was not “the property ought not to change if no methods are invoked”, but more like “the property ought not to change unless the things the object has been doing (such as having methods called) intuitively should change the property”.
The mass property of a car should change if you call its setter, it should also change if the car drives somewhere and consumes petrol, but it should not change if you call ToString() or TurnOnIndicator(Indicator.Left) or something, despite those being method calls.
In which case, to me DateTime.Now meets the second criterion, if not the first. Of course something’s been happening that intuitively ought to change the value of the property – time has elapsed.
Can i say your reading like this:
If a property is changed, it must come from property setters or *relevant* method calls.
Which is a stronger one compared to my reading?
Of course I agree with the constraint you added, but I’m not convinced by the original sentence not because it’s not strong enough, but because it’s no true.
What I suggest is that a property change does not necessarily come from any user code operating the object that, it may also come from a black box, should it be outside the program. or 3rd party code.
Can i say your reading like this:
If a property is changed, it must come from property setters or *relevant* method calls.
Which is a stronger one compared to my reading?
If I understand you correctly, you’re reading of the criterion is that the property should only change if setters or methods are called. My reading is stronger in the sense that it should only be relevant method calls, but weaker in the sense that it might not be method calls at all, but could also be the influence of external systems, provided they’re relevant. Like your last paragraph.
At least that’s how I read “if you drive it for a while, the fifty kilograms contributed by the mass of the gasoline might go down”. In some software that models a car, at least in this decade it’s not the software that’s driving the car, but an external system, the driver.
Wow, then we share the view. I just think if Eric mean the same by the Car example, he would not probably be against the idea of DateTime.Now.
Pingback: Dew Drop – May 20, 2014 (#1780) | Morning Dew
What do you think of Nullable.Value?
This throws an exception, but it a rather helpful property. The alternative would have been a method GetValue(); or neither: to only allow casting — but sometimes the typename is large which makes this option unpleasant.
I seriously dislike Nullable in many ways. Beyond the fact that the read-only property “Value” can throw exceptions, the design of the class often ends up forcing extra data copying operations in both supplier and consumer code.
Consider, for example, the following two methods–one using a MaybeValid[T] and the other a Nullable[T]:
Nullable[T] ValueIfAny(string key)
{
T value;
if (theDict.TryGetValue(key, ref value)
return value;
else
return null;
}
versus
public struct MaybeValid[T]
{ public T Value; public bool IsValid; }
MaybeValid[T] ValueIfAny(string key)
{
MaybeValid[T] result;
result.IsValid = theDict.TryGetvalue(key, ref result.Value);
return result;
}
I know some people dislike aggregate types (simple exposed-field structures), but in cases where what’s really needed is a group of variables bound with duct tape, they can be much more efficient than structures that pretend to be objects.
Very good one.
The simple rule I was taught many years ago to determine whether something should be a method or property is: If the something is a noun it’s a property; if it is a verb or verb phrase, it’s a method. If it isn’t clear whether it’s a noun or a verb, you need to go back and get a better understanding of what this something is.
I swear by rules like these, and tend to swear at code that doesn’t follow them.
Reblogged this on Bob Louwe's blog.
Pingback: 5/25/2014 Developer News Roundup | The Puntastic Programmer
Offtopic: @Eric: there is a big discussion going on at https://roslyn.codeplex.com/discussions/topics/5998/c-language-design regarding the upcoming C# 6 features. It’s really pity that you don’t participate there.
Pingback: Les liens de la semaine – Édition #83 | French Coding
Pingback: Developer News - What's IN with the Infragistics Community? (6/2-6/8) - Marketing & PR - Infragistics.com Blog
At this time it looks like Movable Type is the top blogging platform out there right now.
(from what I’ve read) Is that what you are using on your blog?
This design is steller! You certainly know how to keep a reader entertained.
Between your wit and your videos, I was almost moved to start my own blog (well, almost…HaHa!) Great job.
I really enjoyed what you had to say, and more than that,
how you presented it. Too cool!
Very rapidly this web page will be famous amid
all blogging and site-building users, due to
it’s pleasant posts
Pingback: Async properties in interfaces to cater for the possibility of expensive first-time evaluation: Is this a good idea? | Question and Answer