views:

186

answers:

5

This is something that I've always wrestled with in my code. Suppose we have the following code:

public class MyClass {
    private string _myVariable;

    public string MyVariable {
        get { return _myVariable; }
        set { _myVariable = value; }
    }

    public void MyMethod() {
        string usingPrivateMember = _myVariable; // method A
        string usingPublicProperty = MyVariable; // method B
    }
}

Which way is more correct - method A or B? I am always torn about this. Method A seems like it would be minutely faster, due to the fact that it doesn't have to go access a property before getting the real variable. However, method B is safer because if the getter for MyVariable gets business logic added to it, you are safe by always calling it, even if there is no current business logic.

What's the general consensus?

+1  A: 

This would really depend on what you are accessing the property for. Consider the following two scenarios:

Scenario 1: you write a method to provide a common action on the data in the class:

// assume a hypothetical class Position

public class Circle
{
    private int _radius;
    private int _xpos;
    private int _ypos;

    public int Radius { get { return _radius; } }
    public Position Center { get { return new Position(_xpos, _ypos); } }

    public bool PointInCircle(Position other)
    {
         return distance(this.Center, other) < this.Radius;
    }
}

Clearly the behavior of PointInCircle should be the same as if the user executed the code inside it. Therefore, it makes sense to use the public properties.

Scenario 2: you write a method to manipulate the underlying data. A good example of this is serialization. You would want to serialize the underlying data members as opposed to the values returned by property accessors.

antonm
+1  A: 

depends, if you access the property, there might be 'validation' code that is called.

private int timeSinceLastPropertyAccess;

public int TimeSinceLastPropertyAccess
{
   get 
   { 
      // Reset timeSinceLastPropertyAccess to 0
      int a = timeSinceLastPropertyAccess; 
      timeSinceLastPropertyAccess = 0; 
      return a; 
   }
}

Do you want timeSinceLastPropertyAccess to be reset when it is used when inside your class or not?

PostMan
This is pretty bad getter design. Get methods should not modify state.
Anon.
Too True, but it shows that getters can have undesired effects if used.
PostMan
Consider that we're talking about being inside the same class that has these properties. Thus whether the getters are crappy enough to have major side effects is entirely your own choice.
Anon.
There we go, a better example, where it should reset.
PostMan
+9  A: 

Use the property.

I think the property should be wholly responsible for managing that field.

There are plenty of implementations where it won't matter, but there are lots where it does matter -- a lot. Plus, this can be a bit of a pain to track down, because it always looks right.

You'll go wrong calling the property far fewer times than calling the field, and where there are exceptions to this rule, document the rationale.

Jay
An advantage of using the property is that you can set a breakpoint on every read/write. This is especially useful on large, unfamiliar code bases. It allows you to step over large chunks of code in the debugger, but still stop when the property is modified. Very handy.
Mike Thompson
It can also be handy when unit testing with stubs, because you can set a value on the stubbed property, but not on a private member.
Jay
A: 

Just to add one more thing, your example only asked about getters. The other half of this is setters.

Sometimes you will want the object to use the setters and sometimes you would want it to bypass them and just assign the underlying field.

For example, let's say you have a property called IsModified. Which would tell you whenever the object has been modified. You could have all of your setters flip this to true in the event a different value is assigned to one of the underlying fields.

Now if you are hydrating that object (either loading from a db or somewhere else) then you wouldn't want IsModified set. Because, quite frankly, it isn't modified yet. So in that method you use the underlying field names, but in all of the other methods you use the property setter.

Chris Lively
A: 

it depends, do you want to do what the property does? private/public doesn't really matter, its just like calling a function.

as it is, you have really just set up a "function" in anticipation of having to do something whenever that value is accessed or changed.

the problem with doing this is you might come to find you want to do one thing where its accessed in some places, and another when its accessed in other places, so you are still going to have to change all the 'calls' to it in one of the places.

fact of the matter is, if EVERYTHING that access that variable - even the private class functions - does so through the property that just passes through the variable, why bother having the property at all? why not just create the variable called 'MyVariable' then if you find you wanna do something when its changed/accessed, just create another variable called _MyVariable or something, then change MyVariable to be a property for _MyVariable then.

you should think of properties as being just like the accessor() and mutator() functions you used to write, the trick with them was, if you found that you DID want to do some code whenever a variable was 'accessed' you had to change ALL the calls to that variable to use an accessor instead (calling a function, rather than just accessing a member variable), that is why you would create 'default' accessors and matadors, just in case. As I have stated above, you don't have that problem with c# and properties (except in one lame case where you can't write to the sub members of a member if its a property... WHY??)

matt