5

Possible Duplicate:
Should you access a variable within the same class via a Property?

I ran into this recently and was curious if there was some sort of standard for which one you should reference while inside a class.

I mean really it shouldn't make a difference whether you access the member variable directly or go through the property (unless you need to dodge some custom setter code), but I wanted to be sure there wasn't a best practice for it.

partial class MyClass {
    private string foo;

    internal string Foo {
        get {
            return foo;
        }

        private set {
            foo=value;
            // I do other stuff
        }
    }

    public void DoSomething() {
        //Option 1;
        Foo="some string";

        //Option 2;
        foo="some string";
    }
}
Community
  • 1
  • 1
JWrightII
  • 942
  • 11
  • 26
  • In some cases it could be critical that you use the property (singleton patterns may require this) in others it could be completely unimportant. – Stefan H Jan 23 '13 at 17:51
  • 1
    How important is `// I do other stuff`? If you access the member directly, that `other stuff` isn't done. Is that right or wrong thing to do here? Beyond that, it's your/your team's call. – Anthony Pegram Jan 23 '13 at 17:52
  • @O. R. Mapper - Thanks, apparently I need to brush up on my google-fu, tried searching but couldn't find anything even close. – JWrightII Jan 23 '13 at 18:00
  • @Wrightboy: SO displays the other question on the sidebar as the first item in the "Related" section now. But I guess that wasn't visible at the time of writing, either. – O. R. Mapper Jan 23 '13 at 18:01

8 Answers8

4

This shouldn't be a choice you really make. Either the code in the setter is supposed to run, in which case use the property, or it's not, in which case you use the member variable. In most all situations one is right and one is wrong. Neither is always right/wrong in the general case, and it's unusual for it to "not matter".

For example, if the setter code is firing a "changed" event, do you want external objects to be notified that it changed, or not? If you're changing it in response to a previous change, probably not (infinite recursion anyone?) if no, you probably want to make sure it's fired (so that you're not changing a value and not notifying anyone of changes).

If it's just validating that the value being set is valid, then either you know that, in this context, the value is already validated and must be valid, in which case there is no need to validate again; set the property. If you haven't yet validated what you're about to set then you want the validation logic to run, so use the property.

Servy
  • 202,030
  • 26
  • 332
  • 449
  • However, the only reason you can assume the value is valid is because of the validation logic in the property. As soon as you make it acceptable to bypass that validation logic, it's no longer safe to assume that the value is valid. - As far as events: not firing a Changed event when the value has changed is, in a way, a breach of contract. Infinite recursion is usually avoided by an `if` statement within the property (i.e. only fire the Changed event when the value has actually changed). – JosephHirn Jan 23 '13 at 18:25
  • @Ginosaji As to the validation, no, that's not the only way. Consider, the case where you have a function to set the property to it's default value. You *know* that the default value is a valid value, so there's no reason to need to call validation logic when setting it. As for the events, that wouldn't cover the case. I've seen examples where the property changed event handler of a property needs to change that event. It may not cause infinite recursion, but it would at least fire the event twice when it only ought to fire once. There are other solutions to that problem though. – Servy Jan 23 '13 at 18:34
3

This question is quite a lot debated, so there is no obvious answer to the question.

Personally I prefer to access via the property because you might have some validation or conversion code in it. Even though your getters and setters are trivial, they might change in the future.

1

If you wrapped the field foo in the property Foo, you probably did so for a reason (conversion, events, validation, etc). So, generally speaking, the only place you should be referencing the field foo is in the getters and setters for the property Foo. The rest of the code should reference the property Foo.

I'm sure there exists some obscure situation where you would need to bypass the property's getters and setters, and that's certainly okay to do, but such situations would be the exception to the rule.

JosephHirn
  • 720
  • 3
  • 9
  • You would be correct, I used a property because anytime foo is accessed it needs a few things done to it before it is returned (validation/sorting). I do normally access through the property, but as this sorting and such was somewhat expensive, inside the class it's accessed a lot wanted to circumvent this as it wasn't at all needed. Didn't know if I would be violating any rules by doing that. – JWrightII Jan 23 '13 at 18:06
  • I'm not sure I would place an expensive sorting operation in a getter, but under the assumption I would say that's a valid reason to bypass the property. The only time I would say it is strictly incorrect to bypass the property is if it results in code duplication. – JosephHirn Jan 23 '13 at 18:14
  • Yeah, I did consider just making a method GetSortedFoo(), but outside the class I found that every single time it's use it was needed to be sorted, so I figured why not just sort it on the property. – JWrightII Jan 23 '13 at 18:30
0

Option 1 is good practice. because if you use the Option 2, you will lose other stuff when setting the foo value.

Hossein Narimani Rad
  • 31,361
  • 18
  • 86
  • 116
0

I would go with Option 1. If you're setting a variable, you should use the property and not access the variable directly. This is because the property has the extra code you indicated with "// I do other stuff". You wouldn't want to have to repeat this "other stuff" just because you didn't set the property...unless, you don't want to do "other stuff" when you're setting it this time.

Honestly, this is just a theoretical situation, and it would be a lot easier to answer if you give a practical situation where you encounter this problem.

Eric Freeman
  • 61
  • 1
  • 1
  • 7
0

When using the INotifyPropertyChanged interface, using the property is a must, if you wish to update binded objects.

0

If the setter doesn't have a logic there is no point in explicitly declaring the private variable and it's better to use auto-implemented properties:

    internal string Foo
    {
        get;
        private set;
    }

    public void DoSomething()
    {
        this.Foo = "some string";
    }

If the setter has a logic, the private variable should only be used in the setter and never be modified outside of the setter. In any case (and in my opinion :)) a private variable should never appear anywhere else beside the property setter.

Enrico
  • 1
0

Imagine the code like

public partial class HybridPanel: Panel {
    [DefaultValue(BorderStyle.FixedSingle)]
    public virtual new BorderStyle BorderStyle {
        set {
            if(value!=borderStyle) {
                borderStyle=value;
                base.PerformLayout();
            }
        }

        get {
            try {
                return borderStyle;
            }
            finally {
                if(borderStyle!=base.BorderStyle)
                    base.PerformLayout();
            }
        }
    }

    BorderStyle borderStyle=BorderStyle.FixedSingle;
    bool isCollapsed, isAutoSize;
}

In this conext, the property is not only used as a variable, but also other things to do. Access the properties in the same class is NOT considered a bad pratice, further, the complier would suggest that:

A method that is just for access a field without passing arguments, consider define as a property instead.

By the way, you might correct the description of access the member variable directory to be access the member variable directly(that is, access with the fields).

Ken Kin
  • 4,503
  • 3
  • 38
  • 76