Property abuse in C# is a pet peeve of mine. Properties are meant to represent data, and methods are meant to represent actions. When properties are used improperly, you create a breeding ground for misunderstandings and bugs.
Take a look at MSDN’s guidelines on when to use a method instead of a property. See any you’ve broken lately? I do! In fact, there are two I see all the time, and another that is rare and not even all that bad, but gets me all steamed up.
Properties should not return arrays (or List<T>s, either)
The biggest one I see broken is the rule about not having properties return arrays. In fact, I’ve probably broken this myself some time in the last 3 months. I keep forgetting it. MSDN specifically calls out arrays, but its the same problem with List<T>, or dictionaries, or any other collection that can be changed out from under you. The problem is that if the caller of your class isn’t careful (if they, say, retrieve a list from you via property, start treating it like a local variable, and then add or remove an elements), they will unintentionally change your list out from under you. This is especially concerning when you see a read-only property that’s a List<T>. It’s clear that the intention was that the property wouldn’t be changed, but you are in no way protected from that. It might be helpful to know that List<T> has a AsReadOnly() method which returns a ReadOnlyCollection<T>. While I think this is probably the most widespread abuse of properties I’ve run into, I don’t usually see it cause problems, because I think people are generally aware of the risk. Still, think hard about whether or not you really need your list exposed like this. Chances are that you don’t. It’s a bit of a code smell if you’re expecting callers to call YourClass.YourList.Add(), .Remove(), or .Clear(). It breaks The Law of Demeter.
Properties should not cause observable side effects
Properties with side effects are often the cause of the dreaded Heisenbug.
It’s bad enough when setting one property also happens to set another behind the scenes (typically violating the rule that you should be able to set properties in any order and get the same result), but the ones that have caused some painful debugging sessions are when getters cause side effects. This is because the actual act of debugging (in this case, evaluating the property by having the object in the watch window) changes the state of the object. On a previous project, I was trying to solve a NullReferenceException being thrown when a certain web page was accessed. I was going insane though because when I’d run in debug mode, everything worked fine. Where was this null value coming from? What I finally discovered was that a getter on one of the class’s properties was setting the value of a different property of the class. While debugging, I was viewing the class in the watch window, and thus the getter was being called, resulting in the otherwise null property being set. I wish I could say I’ve only seen that happen once. It’s a pretty safe bet that if looking at an object’s properties in the debugger can change the state of the object, you’ve got a problem with your implementation.
Setters should not have greater accessibility than getters (and therefore, you should never have a write-only property)
I don’t know that this one has much potential to cause bugs, but it violates the idea of what a property represents. Because the least severe things are for some reason the most annoying, it’s also my biggest pet peeve in improper property usage. A setter without an equally accessible getter makes no sense. Remember, properties represent data, not actions. A setter with no getter is probably an action. Use a method instead.
