Skip to content

Proposal: Property-Scoped Fields #133

Description

@bondsbw

Property-Scoped Fields

(Ported from dotnet/roslyn#850)

Summary

Allow fields to be scoped within the accessor body of a property or event declaration.

Motivation

Properties often encapsulate constraints on field access or ensure that certain behaviors are invoked. Sometimes it is appropriate for all field access to be performed through the property, including those from within the same class.

These constraints may be unclear or forgotten, leading to subtle bugs during maintenance which are often difficult to diagnose.

Scoping fields to the body of their property/event would allow the designer to more effectively communicate this intent to encapsulate.

Detailed design

Property-scoped fields may be defined above the accessor list within the body of a property or event:

public string MyProperty
{
    string myField;

    get { return myField; }
    set
    {
        myField = value;
        NotifyOfPropertyChange(nameof(MyProperty));
    }
}
public event EventHandler MyHandler
{
    EventHandler handler;

    add
    {
        handler += value;
        Logger.Log("Added handler");
    } 
    remove
    {
        handler -= value;
        Logger.Log("Removed handler");
    }
}

myField and handler are encapsulated within their respective property/event. In each case, both accessors may directly reference the fields. Nothing outside of the property/event scope has direct access to those fields.

Implementation comments from @CyrusNajmabadi:

First, this is just state that's available to this single property that the value is scoped to. In that regard, it's similar to a 'local'. Just a 'local' that is available to all the accessors. As such, this is how i've been designing things from teh language level:

  1. You can have any number of property locals declared with the property.
  2. The property locals can come before/after/interspersed with the actual property accessors.
  3. The property locals are scoped to that property alone. Other members of the class/struct cannot interact with them.
  4. Subclasses can't interact with them (including overrides of the property).
  5. 'var' can be used for these property locals.
  6. Indexers should also be able to use these sorts of property locals.

Now (while this is out of the scope of the actual language change), here's how i would expect to emit them.

  1. These would simply be fields within the same class/struct that the property/indexer is contained within.
  2. The fields would be private.
  3. The fields would be named in such a way that they could never collide with an actual class member. likely something wonky <>_propname_localname

Alternatives

Automatic backing field

This implementation would be an extension of automatic properties. The backing field could be accessed through a keyword field, but only within the scope of the property/event:

public string MyProperty
{
    get { return field; }
    set
    {
        field = value;
        NotifyOfPropertyChange(nameof(MyProperty));
    }
}

This would not allow multiple fields to be encapsulated in the same property.

dotnet/roslyn#7614 has a similar proposal with a different keyword ($state).

Auto-property syntax for custom setter/getter logic

dotnet/roslyn#1551

Semi-auto-properties with setters

dotnet/roslyn#8364

Class-scoped blocks for fields / explicit field usage

dotnet/roslyn#12361

Unresolved questions

A list of questions from @CyrusNajmabadi:

  1. Allow attributes on these locals? It wouldn't be hard to support, and there could be value in putting attributes on the final fields emitted. This does leak through though the implementation of how we deal with property locals.

2. Allow these for events? Yes

  1. How are we going to handle the syntax changes here in our API. It's non trivial to modify the syntactic constructs here in a non-breaking fashion.
  2. We've added 'local functions' to C#7. Should we allow "property local functions" as well?
  3. Should we allow other 'local' constructs? We could have local methods/types/etc. within a property. Technically it would all be possible.

Syntax

It was noted that adding fields above accessors or below accessors (but not interspersed) could be an easier change to the existing compiler design. @lachbaer provided a potential design which would not be a breaking change to the API:

<!-- this Node hase been added, it looks exactly like "FieldDeclarationSyntax" -->
<Node Name="PropertyLocalFieldDeclarationSyntax" Base="BaseFieldDeclarationSyntax">
    <Kind Name="PropertyLocalFieldDeclaration"/>
    <Field Name="AttributeLists" Type="SyntaxList&lt;AttributeListSyntax&gt;" Override="true">
      <PropertyComment>
        <summary>Gets the attribute declaration list.</summary>
      </PropertyComment>
    </Field>
    <Field Name="Modifiers" Type="SyntaxList&lt;SyntaxToken&gt;" Override="true">
      <PropertyComment>
        <summary>Gets the modifier list.</summary>
      </PropertyComment>
    </Field>
    <Field Name="Declaration" Type="VariableDeclarationSyntax" Override="true"/>
    <Field Name="SemicolonToken" Type="SyntaxToken" Override="true">
      <Kind Name="SemicolonToken"/>
    </Field>
  </Node>

  <Node Name="AccessorListSyntax" Base="CSharpSyntaxNode">
    <Kind Name="AccessorList"/>
    <Field Name="OpenBraceToken" Type="SyntaxToken">
      <Kind Name="OpenBraceToken"/>
    </Field>
    <!-- next line has been added -->
    <Field Name="LocalFields" Type="SyntaxList&lt;PropertyLocalFieldDeclarationSyntax&gt;" Optional="true" />
    <Field Name="Accessors" Type="SyntaxList&lt;AccessorDeclarationSyntax&gt;"/>
    <Field Name="CloseBraceToken" Type="SyntaxToken">
      <Kind Name="CloseBraceToken"/>
    </Field>
  </Node>

Field name collisions

Should a property-scoped field be allowed to have the same name as another class member?

Should local identifiers within accessors be allowed to have the same name as a property-scoped field?

Design Meetings

Activity

  1. changed the title [-]Property-Scoped Fields[/-] [+]Proposal: Property-Scoped Fields[/+] on Feb 17, 2017
  2. scottdorman commented on Feb 17, 2017

    @scottdorman
    Contributor

    Overall, I like this. Would it be reasonable to implement this so that we have both property scoped fields and also the automatic backing field?

    For simple cases, such as properties that just need to implement INotifyPropertyChange we don't need to declare the field ourselves:

    public string MyProperty
    {
        get { return field; }
        set
        {
            field = value;
            NotifyOfPropertyChange(nameof(MyProperty));
        }
    }

    For more complex scenarios, we can declare our own local property scoped field:

    public string MyProperty
    {
        string myField;
    
        get { return myField; }
        set
        {
            myField = value;
            NotifyOfPropertyChange(nameof(MyProperty));
        }
    }

    Should a property-scoped field be allowed to have the same name as another class member?

    Yes, I think so. Would this really be any different than having a variable defined inside a method having the same name as variable defined outside the method?

  3. Thaina commented on Feb 17, 2017

    @Thaina

    Totally agree

    But I would disagree to allow name collision. It should not collide with any other field

  4. bondsbw commented on Feb 17, 2017

    @bondsbw
    Author

    this. can be used to access class members which are hidden by locals. Presumably it would also be used for property-scoped field access. But if name collisions were allowed, then it would be possible to have property-scoped fields which are inaccessible when hidden by locals (or, a different keyword or syntax would be needed to access them).

  5. soyeyalon commented on Feb 17, 2017

    @soyeyalon
    Contributor

    @scottdorman I guess that for simple scenarios and probably even complex ones you could use source generators for this so you wouldn't have to do that manually.

    [NPCGenerator]
    public string MyProperty { get; set; }
    
  6. lachbaer commented on Feb 17, 2017

    @lachbaer
    Contributor

    @eyalsk But not everybody has those generators by hand, especially when reading foreign code.

    I would also disagree to allow name collision, with the exception of modifying a local 'new'.

    public string MyProperty
    {
        string myField;
    
        get => {
            new int myField;
            /* why should you do this, it doesn't make sense, but it assures that you know what you do */
        }
        set => myField = value;
    }
  7. scottdorman commented on Feb 17, 2017

    @scottdorman
    Contributor

    All good arguments, I think, for not allowing the name collisions. I'm changing my vote to "a property-scoped field should not be allowed to have the same name as another class member."

    @eyalsk Yes, a code generator would work but not everyone can (or wants to) use them. Personally, I think access to the backing field of an autoprop should have been there from the start. Also, code generators aren't necessarily reasonable for all scenarios where I'd want access to the backing field. A few examples of that is doing complex validity checks on the value before setting it or setting the property has side-effects of setting some other read-only properties (think about splitting a full name property up in to first and last name).

  8. lachbaer commented on Feb 17, 2017

    @lachbaer
    Contributor

    In the current C# 7 preview in Local Functions (see here) when you re-declare a local variable you will get the following error:

    A local or parameter named 'localVariable' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter (ERR 0136)

            static void Main(string[] args)
            {
               var localVariable = 12;
    
                void LocalFunction()
                {
                    var localVariable = 12;  // ERROR, duplicate declaration
                }
            }

    The same error could be issued when using a property scoped local - where the whole property is the enclosing scope, thus eliminating any further confusions or needs for extra keywords to access them (the outer variable, not the confusion 😉).

  9. soyeyalon commented on Feb 17, 2017

    @soyeyalon
    Contributor

    @scottdorman, @lachbaer Im not sure I understand you because it would probably be just a Install-Package <SomePackage> away.

    I like the original proposal but I think that the amount of ways that you can construct properties today is overwhelming so going further with it and add more ways is just taking properties to the extreme. :)

  10. soyeyalon commented on Feb 17, 2017

    @soyeyalon
    Contributor

    @scottdorman I think that you need to create a new proposal for it, any sub topic should have its own proposal.

  11. scottdorman commented on Feb 18, 2017

    @scottdorman
    Contributor

    @eyalsk Looks like @lachbaer already created the proposal for it. I brought it up here because I was responding to the first two listed alternatives in this proposal, but I agree it needs to be a separate proposal.

  12. HaloFour commented on Feb 21, 2017

    @HaloFour
    Contributor

    I think it's fine for two property-scoped fields to share the same name, but not for a property-scoped field to share the same name as another top-level member:

    // good
    public class FizzBuzz {
        public string Fizz {
            string field;
            get => field;
            set => field = value;
        }
    
        public string Buzz {
            string field;
            get => field;
            set => field = value;
        }
    }
    
    // bad
    public class FizzBuzz {
        private string field;
        public string Fizz {
            get => field;
            set => field = value;
        }
    
        public string Buzz {
            string field; // CS0102
            get => field;
            set => field = value;
        }
    }

    I'm not a fan of partially-auto-implemented properties. With expression-bodied accessors much of the syntactic boilerplate is already eliminated. Having to declare an explicit fields seems reasonable, especially in the case of dealing with calculated values (e.g. name parsing).

  13. lachbaer commented on Mar 7, 2017

    @lachbaer
    Contributor

    If there is only one field needed for the primary use as backing field, I suggest an additional syntax, borrowed from C struct:

    public string FirstName
    {
        get => _name;
        set => _name = value;
    } _name;

    _name is by definition private to it's type (maybe unless directly prepended by an access modifier?).
    OR, appropriate for this proposal, just scoped to the corresponding property.

    The advantage of this syntax would serve 2 purposes:

    1. no need to explicitly state the type of the property (again)

    2. This would also be in favor of Proposal: field keyword in properties #140, Semi-Auto-Properties, because that named backing field would expunge the need for a field keyword:

    public string FirstName
    {
        get;
        set => _name = value;
    } _name = "(not set)";

    or even shorter, in case expression-bodied set of semi-auto-properties expect a value being returnd to assign to the backing-field:

    public string FirstName
    {
        get;
        set => value;
    } _name = "(not set)";
  14. 39 remaining items

  15. HaloFour commented on Oct 9, 2020

    @HaloFour
    Contributor

    I think one thing that could make this proposal much more useful would be to lift the limitation on the property-scoped field initializer and allow it to access instance members. That would enable scenarios like this following:

    public int LazyProp {
        Lazy<int> lazy = new Lazy<int>(ExpensiveComputation);
        get => lazy.Value;
    }

    Even better if the property-scoped field could be treated like a static local of sorts and can have its type inferred by the initializer:

    public int LazyProp {
        var lazy = new Lazy<int>(ExpensiveComputation);
        get => lazy.Value;
    }

    This also feels like a stepping-stone towards delegated properties and could simplify a number of AOP-like scenarios.

  16. bondsbw commented on Oct 9, 2020

    @bondsbw
    Author

    @HaloFour I like it, but it seems like the compiler could run into a dependency cycle if multiple property-scoped initializers reference each other’s property getters.

  17. modified the milestones: , 10.0 Working Set on Oct 14, 2020
  18. TahirAhmadov commented on Nov 10, 2021

    @TahirAhmadov

    How would the event be raised in this scenario? If an event defines the add/remove, the current convention is that the underlying field is used to raise the event, not the event itself.
    I think a solution would be to add a new accessor to events - get:

    public event EventHandler Foo
    {
      EventHandler foo;
      get => foo;
      add => foo += value;
      remove => foo -= value;
    }
  19. TahirAhmadov commented on Nov 10, 2021

    @TahirAhmadov

    @jnm2 Actually, the problem with your solution is this:

    bool _active, _dead;
    public event EventHandler Foo
    {
      EventHandler fooActive, fooInactive;
      add { if(!this._dead) { if(this._active) fooActive += value; else fooInactive += value; } }
      remove { if(!this._dead) { if(this._active) fooActive -= value; else fooInactive -= value; } }
    }

    What would this.Foo(this, EventArgs.Empty) raise in this case?

  20. HaloFour commented on Nov 10, 2021

    @HaloFour
    Contributor

    I'd rather a raise accessor than a get accessor. A get accessor would leak the details of the underlying publisher, which might not even be a delegate of the type of the event. A raise accessor would hide that detail behind a method of a signature matching that delegate.

    That assumes that scoped fields make sense in the case of events, and I'm not really sure that they do. It's extremely niche to explicitly implement an event as it is.

  21. modified the milestones: Working Set, Backlog on Sep 26, 2022
  22. CharlesJenkins commented on Feb 13, 2023

    @CharlesJenkins

    Is there a way to vote to advance this proposition? So often I have felt guilty about creating private fields that should never be manipulated outside of Property accessors. I know it's wrong to put them in scope to trip up a maintainer, but the language currently gives us no alternative!

  23. CyrusNajmabadi commented on Feb 13, 2023

    @CyrusNajmabadi
    Contributor

    @CharlesJenkins Yes. Vote here:

    image

  24. CharlesJenkins commented on Feb 14, 2023

    @CharlesJenkins

    @CharlesJenkins Yes. Vote here:

    image

    Thanks! I hope I got the right one.

  25. locked and limited conversation to collaborators on Nov 19, 2024
  26. converted this issue into a discussion #8632 on Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions