Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free.

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?
share|improve this question
    
See this question and answer. –  Peter K. Aug 16 '12 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 Aug 16 '12 at 18:52
    
@KChaloux: Understood! That's why it's a comment, not an answer. :-) –  Peter K. Aug 16 '12 at 19:30
    
@PeterK. Fair 'nuff =p –  KChaloux Aug 16 '12 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 Mar 5 at 12:53

4 Answers 4

up vote 11 down vote accepted

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.

share|improve this answer
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 Aug 16 '12 at 18:58
2  
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 Aug 16 '12 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 Aug 16 '12 at 20:56
    
A tool like Fody can implement INotifyPropertyChanged without the need for explicit backing fields. github.com/Fody/PropertyChanged –  Sebastian Redl May 13 at 12:35

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.

share|improve this answer
    
I've actually done that on a few of the old fields when refactoring code... it's a pain. –  KChaloux Aug 16 '12 at 18:56

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

share|improve this answer
2  
#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 May 13 at 14:48

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.

share|improve this answer
6  
You shouldn't use public fields. –  svick Aug 16 '12 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 Aug 16 '12 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. –  Kevin McCormick Aug 16 '12 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 Aug 16 '12 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. –  Kevin McCormick Aug 17 '12 at 17:46

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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