Backing field scoped to Property

[john-cd]

I would like to propose a solution to a common bug - misuse of private fields backing a property.

private string _field = 1;    // accessible to all methods in the class.
public event EventHandler Warn;

        public string Prop
        {
            get
            {
                return _field;
            }
            set
            {
                _field = value;
                Warn?.Invoke(this, new EventArgs());
            }
        }

_field, while intended to be set only in the property, can be used by mistake in other parts of the code, resulting in missing events. Similar issues exists with update of multiple fields by a property, lock-protected fields, etc... Fields scoped to a property should be implicitly private, and could be marked readonly if the set accessor is not present. Static fields should be supported for WPF usage.

I propose to allow the declaration and scoping of the field inside the property's body:

        public int Prop
        {
            int _field = 1;    // accessible only in get / set; should not be marked private / protected / public
            get
            {
                return _field;
            }
            set
            {
                _field = value;
                Warn?.Invoke(this, new EventArgs());
            }
        }

       // not OK
      public void foo()
      {
        _field = 42;
      }

To provide access from the constructor only, consider allowing

public readonly string A              // property flagged readonly
{
   readonly string _field = "foo";   // or inner field flagged readonly
  get 
  {
     return _field;
  }
  private readonly set                  // or set accessor  flagged readonly
  {
    _field = value;        // accessible only from the constructor
  } 
}

public MyType()
{
this.A = 42;  // OK
}

doStuff() { A = 42; }   // Not OK

In order to simplify immutable types, also consider allowing

class C
{
  public C(int prop)
  {
    Prop = prop; // assignment in the constructor only
  }

  public int Prop { get; readonly set; }  // syntax more clear and explicit that current:  public int Prop { get; }

  doStuff() { Prop = 42; }   // Not OK
}

[maniero]

Duplicate of #140?

[MgSam]

Duplicate of #133

[john-cd]

To a degree - I have not seen discussion of "readonly" in that context.

Whatever design is chosen should allow multiple backing field for a single property.

I should provide two sub-scenarios / alternative designs for access from the constructor:

// 1) hydrate the field, bypassing change events, argument check... 
public class C
{

  public C()
  {
     _field = 42;       // initialize with a value not normally allowed 
  }

public string Prop             
{
   readonly string _field = "foo";   // inner field flagged readonly thus accessible from the constructor
  get 
  {
     return _field;
  }
  private set
  {
     if (value == 42) throw new ArgumentException("Prop");
    _field = value;       
    Warn.Invoke(this, new EventArgs());  // warn for changes only, not during construction
  } 
}

// 2) Enforce argument checking always...
  public C()
  {
     Prop = 42;
  }

public string Prop             
{
  string _field = "foo";   // inner field NOT flagged readonly thus NOT accessible from the constructor
  get 
  {
     return _field;
  }
  readonly set  // set only accessible from constructor
  {
    EnforceSomethingAlways(value);
    _field = value;       
  } 
}

[HaloFour]

@john-cd

How do you propose that the compiler translate that into something legal?

[HaloFour]

Your last example is already possible:

class C
{
  public C(int prop)
  {
    Prop = prop; // assignment in the constructor only
  }

  public int Prop { get; }  // implicitly readonly

  void doStuff() { Prop = 42; }   // Not OK
}

[john-cd]

@HaloFour in a way similar to what the compiler already does for auto properties. I don't recall - I think for autoprop it creates a private property with a mangled name and decorate it with an attribute to prevent access from outside the getter and setter.

[HaloFour]

@john-cd

So despite the use of the readonly keyword the backing field is not actually readonly and the behavior is only enforced by the compiler?

[john-cd]

That or the compiler could spit out something like

[NotAccessibleExceptFromGetterSetterAndConstructor]
private readonly int _field; 
public int Prop { get { return _field } set { _field = value; }}

Note that currently auto prop are basically syntactic sugar; I think you still can access the hidden backing field using Reflection.

A stronger but heavier implementation would generate something akin to

class C 
{ 
  private readonly InnerClass _ic;
  public C()
  {
     _ic = new InnerClass(42);
  }

  public int Prop { { get { return _ic.Prop; } set { _ic.Prop = value; } }

  private class InnerClass
  {
    public InnerClass(int field)  
    {
    Prop = field;
    }
    public Prop {  get;  } 
  }
}

or the equivalent using a singleton shared among all class instances.

Alternatively, the CLR could introduce the notion of "scope within a class" e.g.

class C 
{     
  // inner scope, not an object
  {
  private int _field;
  public int Prop { ... }
  }
}

2. The last example is indeed possible - though the current design is a bit of a code smell, in the sense that, without referring to the constructor, it is hard to tell that the property can / is indeed initialized.

[HaloFour]

@john-cd

Auto-properties are just compiler candy, but readonly fields are enforced by the CLR, at least to the extent that an assembly that attempts to write to one directly is considered unverified.

[DavidArno]

Aside from slightly less syntax, what benefit would this really bring over what's already possible now:

class SomeThing
{
    public event EventHandler Warn;
    private readonly PropWrapper _field = new PropWrapper();

    private class PropWrapper
    {
        private string _field = "1";

        public string Prop
        {
            get => _field;
            set
            {
                _field = value;
                Warn?.Invoke(this, new EventArgs());
            }
        }
    }

    public string Prop // rest of SomeThing class has to use this to affect the real _field value 
    {
        get => _field.Prop;
        set => _field.Prop = value;
    }
}

[john-cd]

Simplified syntax is the point - properties that validate the input or send a change notification are extremely common. Introducing an explicit wrapper means doubling the number of objects in the heap, adds a level of indirection and dirties OO encapsulation.

I always thought that auto properties via { get; set; } are a bit of an anti pattern. Properties are not meant to be a substitute for a public field; they are meant to be a generalization of a method (or grouping of related methods acting on the same field(s). The syntax should encourage that view.

[DavidArno]

Simplified syntax is the point - properties that validate the input or send a change notification are extremely common. Introducing an explicit wrapper means doubling the number of objects in the heap, adds a level of indirection and dirties OO encapsulation.

OK, I can understand that point, though I disagree that it "dirties" encapsulation: it enforces it. However I'd consider the whole approach of denying a type access to its own internals as a likely "the class is too big to keep track of things" smell.

I always thought that auto properties via { get; set; } are a bit of an anti pattern.

I agree, but for different reasons to you: I'm very much a fan of just { get; } so they serve as read-only views onto the underlying data.

[MouseProducedGames]

@DavidArno To chime in here, I understand your point there and mostly agree with it; however, on this single point, there are cases where you have to grit your teeth and make a large class or struct; or use globals, or...

[john-cd]

On a larger level, the issue is that

• C# provides coarse-scoped scopes (namespaces, classes) and fine-grained (methods, local functions, using statement), but no intermediate scope within a class except auto properties (with their limitations) and events (same issue).
• The scopes are often tightly tied to memory handling (heap -> classes, stack -> methods) or resource management (using / IDisposable).

This is problematic for feature-rich classes (typical in UI work - remember Winforms) with a lot of properties and events and for partial classes (leakage of the details of the auto-generated implementation into the custom code). That means we are forced to use aggregation / composition in a way that is counter-intuitive; hence the proliferation of "Settings" and "Info" classes in the framework.

Composition semantics should signify "has-a" relationships ( "a car has wheels" ), not be used to artificially encapsulate something that has an intrinsic "is-a" relationship ("the car's color is red").

The proposal above segways into the discussion around traits with properties: #288 Curious to hear from proponents how it all could tie together?

[gafter]

/cc @CyrusNajmabadi

[CyrusNajmabadi]

Yup. I've been following this :)

[john-cd]

On second thought, here is a generalization of the property syntax that covers the above issues:

public T Prop 
{
  T _field;
  // more than one inner field should be possible (e.g. _isInitialized, _lock...)
  get => _field;
  set { Validate(value);  _field = value; MyEvent?.Invoke(this, new EventArgs()); } 
  init {  Validate(value);  _field = value; }  // for constructor and autoprop initializer use only; private by    default; perhaps protected? 
}

where the current syntax

T Prop { get ; } = somevalue;

has an implicit init => _backingfield = value; and the current constructor syntax

public C(T prop) 
{
 Prop = prop;
}

is a call to init, inlined to an assignment to the field by compiler if possible.

That syntax would bypass the not-quite-readonly readonly non-sense and treat inner fields as pseudo-locals - to the property or event, not a function.

Also, object initializers like

var c = new C() {  Prop = someValue  }; 

would call init if set is not available e.g. for immutable types. As discussed on another thread, that could translate into a call to an auto-generated constructor.

You probably don't want inner fields in interfaces or traits (interfaces with default methods / properties, under discussion) because of diamond inheritance issues.

interface IBehavior
{
    T Prop { get; set; protected init; }  // no inner field here but init declaration allowed
}

public class C : IBehavior
{
 T Prop 
 { 
 T _field;
 get =>Transform( _field);
 set => _field = ValidateAndSendEvent(value);
 protected init => _field = Validate(value);
 }
}

although it might be possible to provide partial implementation of one or more getter / setter / initializer in the trait (?).

You might also consider allowing local functions in properties e.g.

public T Prop
{
  T _field;

  get =>Transform( _field);
  init => _field = Validate(value); 

  T Transform(T val) { return val + 42; } 
  T Validate(T val) => val ?? throw new ArgumentNullException("Prop");
}

and in traits:

interface IBehavior
{
    T Prop 
    {
    get;
    set; 
    protected init;
    T Validate(T val) => val ?? throw new ArgumentNullException("Prop");  // default impl
    } 
} 

class C : IBehavior
{
 public T Prop
 {
  T _field;
  get =>Transform( _field);
  init => _field = Validate(value); 

  T Transform(T val) { return val + 42; } 
  overrides T Validate(T val) => val ?? 42;  // implementation override if needed
 }
}

Thoughts?

[HaloFour]

is a call to init, inlined to an assignment to the field by compiler if possible.

init could not be a real method. Whatever it did would have to be inlined directly into any constructor that called it. Otherwise the assembly would be unverifiable. Due to this I don't see how you could have an init method in something like a trait.

[john-cd]

@HaloFour init would behave like get or set - a property would have up to three entry points instead of two.

I don't know enough about the proposed traits implementation to judge, but feel free to explain.

[HaloFour]

@john-cd

So how does that bypass the "not-quite-readonly readonly" nonsense? Nothing may assign to a readonly field except the constructor.

[john-cd]

@HaloFour

I was initially proposing to allow the readonly keyword on the backing field or the property or the property setter to allow initialization from the constructor, but the syntax is confusing at best.

Treating the backing field as a pseudo-local (no readonly, private, protected allowed) and adding a third property accessor (init) is a cleaner, more readable way.

Instead of using readonly to bypass the setter and access the backing field directly in the constructor, the proposed syntax formalizes the fact that there are three ways to access a property ( get, set, initialization ) and transfer the responsibility for value validation during initialization from the caller (the constructor or object initializer) to the callee (the property).

[jnm2]

I prefer #140 to this because it covers just about all my use cases and is cleaner.

// field is a contextual keyword just like value
public bool IsEditing { get; set => RaisePropertyChanged(ref field, value); }