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 of
DateTimes, and it changes frequently on its own. In retrospect, it ought to have been a
- 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 the
ReadOnlyCollectionwrapper 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.