views:

172

answers:

8

I am reviewing another developer's code and he has written a lot of code for class level variables that is similar to the following:

    /// <summary>
    /// how often to check for messages
    /// </summary>
    private int CheckForMessagesMilliSeconds { get; set; }

    /// <summary>
    /// application path
    /// </summary>
    private string AppPath { get; set; }

Doesn't coding this way add unnecessary overhead since the variable is private?

Am I not considering a situation where this pattern of coding is required for private variables?

+6  A: 

Private properties provide you a level of abstraction when you assign the value to the private variable (which is created for you by the compiler). It's not less efficient to do so, and if you need to change how the assignment is done in the future, you only have to worry about updating it in one place.

It can also provide consistency in an application. If all assignments are done through properties, then some can validate the assignment values while others not. To the person interfacing with the property, there is no difference between those which validate and those which do not. (and this way someone doesn't accidentally assign a value to a local variable which doesn't validate.)

Kevin
Most of the time the JIT compiler will optimize the calls to the getter and setter away, so I wouldn't worry much about efficiency.
Steven
+1  A: 

IIRC, the compiler will optimize the access to the properties and it will end up to be direct references to the automatically generated backing field.

This will result in no overhead, cleaner and more maintainable code.

João Angelo
It would be the JIT compiler, not the C# compiler.
Jon Skeet
+8  A: 

That's like saying private methods aren't beneficial since they add unnecessary overhead and no one outside the class is going to use them.

Properties give you one point of contact between a variable and the rest of your code. In the future, you might want to add error checking or update some other value whenever the variable changes, and properties will let you do that.

MikeP
+1: This pattern has a name: **self-encapsulated field**. It makes sense when enforcing the rules or invariants associated with a field would otherwise become duplicated and spread out. There's an excellent article about it here: http://sourcemaking.com/refactoring/self-encapsulate-field
LBushkin
A: 

check out this link:

http://social.msdn.microsoft.com/Forums/en-US/csharplanguage/thread/3f580715-9d84-4468-92a1-7cf4831aa3f6

They are discussing about the same issue.

HTH

Raja
+3  A: 

No, the jit compiler will convert property accesses into direct variable accesses on the backing field, so it's no less efficient than using a member variable directly.

The advantage is that if you ever wish to convert the property into a nontrivial implementation, you can just add code to the get/set to allow additional checks or behaviours or a different storage mechanism to be used for a property without having to refactor any of the client code (in this case, all in the same class).

Jason Williams
+3  A: 

I would say it is a good practice to be in the habit of using properties instead of fields.
The overhead is likely to be minimal to nonexistent, since the JIT can inline the getter and setter. So while not necessarily required, there's nothing wrong with doing it this way.

Daniel Plaisted
A: 

The performance issue is probably negligible or non-existent, but it requires you to type 9 more characters (+ spaces) for no gain. If you later want to convert your fields to properties, you can do it anyway because fields and properties are fully source compatible (as long as you don't use mutable value types).

erikkallen
A: 

I don't see any real benefit to this, though I don't see any harm either. If you are going to convert the property into a public thing in the future, it may save you a little typing. But how often do you convert private members into public properties? And when you do, it is probably just one or two items out of however-many members your class contains. If you are going to convert to a non-trivial implementation or add any kind of validation, you will need to convert the auto-implement property into one with a real backing field, which negates any simplicity benefit you might have thought you gained at the start.

To me it seems like a personal preference thing, with no performance impact, and some minor impact (both positive and negative) when making future changes.

Ray