21

The codebase I'm working in now has the convention of using private fields and public properties. For example, most classes have their members defined like this:

// Fields
private double _foo;
private double _bar;
private double _baz;

// Properties
public double Foo
{
    get{ return _foo; }
    set{ _foo = value; }
}

public double Bar
{
    get{ return _bar; }
    set{ _bar = value; }
}

public double Baz
{
    get{ return _baz; }
}

I know these can be rewritten without their internal private properties:

public double Foo{ get; set; }
public double Bar{ get; set; }
public double Baz{ get; private set; }

I'd like some input on this:

  • Is there a good reason to prefer the older, more explicit style over the newer, more concise one?
  • Should I write any new classes using the concise style, or should I try to match the older code for consistency? Is consistency worth enough in this case to justify the older format?
5
  • See this question and answer.
    – Peter K.
    Commented Aug 16, 2012 at 18:45
  • @PeterK. Informative. Doesn't answer whether or not I should worry about keeping with the style of the rest of the program, or if it's a small enough detail not to matter though.
    – KChaloux
    Commented Aug 16, 2012 at 18:52
  • @KChaloux: Understood! That's why it's a comment, not an answer. :-)
    – Peter K.
    Commented Aug 16, 2012 at 19:30
  • @PeterK. Fair 'nuff =p
    – KChaloux
    Commented Aug 16, 2012 at 19:44
  • 1
    another reason not to change is if anything is passed over the wire with WCF - automatic properties have contract issues (due to the invalid characters in the backing field names created by the .NET)
    – Leon
    Commented Mar 5, 2015 at 12:53

5 Answers 5

21

There are a couple instances where the so-called "older" style is still required:

A: Immutable types using language-provided immutability. The readonly modifier in C# freezes that value after construction. There's no way to mimic this with automatically-implemented properties (yet).

public sealed class FooBarBazInator
{
    private readonly double foo;

    public FooBarBazInator(double foo)
    {
        this.foo = foo;
    }

    public double Foo
    {
        get
        {
            return this.foo;
        }
    }
}

B: Your getters/setters have any logic whatsoever. WPF and Silverlight (and similar) code that are data-bound to your classes will implement INotifyPropertyChanged like so:

public class FooBarBazInator : INotifyPropertyChanged
{
    private double foo;

    public event PropertyChangedEventHandler PropertyChanged;

    public double Foo
    {
        get
        {
            return this.foo;
        }

        set
        {
            this.foo = value;
            this.RaisePropertyChanged("Foo");
        }
    }

    private void RaisePropertyChanged(string propertyName)
    {
        var propertyChanged = this.PropertyChanged;

        if (propertyChanged != null)
        {
            propertyChanged(this, new PropertyChangedEventArgs(propertyName));
        }
    }
}

Other than those, I'd say use the new style. Keeps your code concise and offers less surface area for bugs to creep in. Plus, if it ever needs to change to include logic, it won't be signature-incompatible.

5
  • 2
    Everyone seems to be in agreement about going with the new style. You get a +1 and the answer credit for telling me about the necessity of a field with the readonly modifier, which I didn't know. =D
    – KChaloux
    Commented Aug 16, 2012 at 18:58
  • 3
    Some (including me) consider auto properties with private accessor “good enough” for immutable properties. True, they may not be truly immutable, but you just have to be careful not to use the setter outside of constructor.
    – svick
    Commented Aug 16, 2012 at 19:01
  • 2
    another reaon is if you want to pass the value as ref which does not work with properties, so you need the field
    – stijn
    Commented Aug 16, 2012 at 20:56
  • 1
    A tool like Fody can implement INotifyPropertyChanged without the need for explicit backing fields. github.com/Fody/PropertyChanged Commented May 13, 2015 at 12:35
  • 3
    Take note: C#6 allows for "getter-only auto-properties" which provides in a single line what my example "A" is doing. Commented Aug 12, 2016 at 14:23
8

I would say that having explicit backing field is just useless cruft: it makes your code longer and harder to read. It also adds potential of error:

public double Bar
{
    get { return _baz; }
    set { _bar = value; }
}

I think an error like this could happen quite easily and would be quite hard to spot.

And if you want consistency, do it the other way: change code that uses backing fields into code that uses automatic properties. This is especially easy if you use a refactoring tool like ReSharper (and if you don't use something like that, I think you should start).

Of course, there are still cases where having backing field is necessary, but I think that's not relevant to this question.

1
  • 1
    I've actually done that on a few of the old fields when refactoring code... it's a pain.
    – KChaloux
    Commented Aug 16, 2012 at 18:56
0

Even though the question is quite old I thought of adding couple of points which I read from a book worth mentioning here.

  1. The run-time serialization engines persist the name of the filed in a serialized stream. The name of the backing field for an Automatically implemented property ( AIP )is determined by the compiler, and it could actually change the name of this backing field every time you recompile your code, negating the ability to deserialize instances of any types that contain an AIP. So do not use AIP with any type you intend to serialize or deserialize .

  2. When debugging, you cannot put a breakpoint on an AIP get or set method, so you cannot easily detect when an application is getting or setting this property. You can set breakpoints on manually implemented breakpoints

1
  • 3
    #1 - Most serializers ignore private properties/fields by default; I've never encountered the issues you mention. #2 - This is fixed in VS2015 and can be worked around in VS2013. See this Visual Studio Magazine article.
    – Brian
    Commented May 13, 2015 at 14:48
-1

Properties without fields are better for readability but if the class is decorated with a [Serializable] attribute then you should keep the backing field.

Note also that with VS 2019 the "Use auto property" quick action is disabled for the same reason.

-4

Is there a good reason to prefer the older, more explicit style over the newer, more concise one?

Not really. Though I would argue that any time you had a pass through property like that, you should've just used a public field to begin with.

Should I write any new classes using the concise style, or should I try to match the older code for consistency? Is consistency worth enough in this case to justify the older format?

Assuming you ignored the above advice and have passthrough properties, I would not aim to match the style. Ideally, you would go back and refactor the old style into the new style to be consistent, but the chance of people misreading that code is slim.

7
  • 9
    You shouldn't use public fields.
    – svick
    Commented Aug 16, 2012 at 18:58
  • @svick - Bah. Unless you're doing reflection into the type there's no difference. If you're publically exposing the type as a service or library interface, you're going to expose an interface which is going to require a property anyways. The OP is not doing any of those things.
    – Telastyn
    Commented Aug 16, 2012 at 20:41
  • 2
    There is no technical reason in these limited cases. Don't forget properties behave differently in the debugger, class diagrams, intellisense, and just when you think you're not doing reflection, the .NET framework is. WinForms, WPF, and several other components internally use reflection and they do treat properties separately, so the advice to use public fields, ever, will be poorly received by lots of devs on this site. Commented Aug 16, 2012 at 23:30
  • 1
    @KevinMcCormick and the framework behaves perfectly well (from what I understand) with fields. It doesn't really matter since there's auto-properties now, but the bigger problem is making the things public in the first place. Far, far too many people think that simply making things properties somehow absolves them from 'don't make public fields'.
    – Telastyn
    Commented Aug 16, 2012 at 23:53
  • 2
    The Framework handles them differently: try using a public field in EF POCO, bind it to a DataGridView, use a PropertyGrid, etc. I just did a quick search in my decompiler to Type.GetProperties/GetProperty and the Framework makes calls to this method hundreds of times, but not to GetField. Bottom line, they are different, and the recommendation to always use properties for public data is based partly on this fact. This doesn't preclude using fields ever, but that case needs justification. However, your other point is definitely true, exposing public data carelessly is always a bad idea. Commented Aug 17, 2012 at 17:46

Not the answer you're looking for? Browse other questions tagged or ask your own question.