-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposal: Property-scoped fields #850
Comments
I think instead of
must be used
but the compiler must ensures no setter method will be called. Just assignment to a field. Like in Swift. Because using of setter in constructor may lead to unexpected side effects |
Agreed that calling setters in constructors are generally the non-preferred case. I'm just worried that using the |
I disagree adding a new keyword, first it will limit one property has only one field, or why this field has a special name when others maybe also affects its return value ( Besides I think it's also ok to set the initial value to a custom-setter property with C# 6 syntax ( |
@CnSimonChan:
You highlighted the main problem with the |
@bondsbw I think about confusion. What if write in constructor like this
This makes no confusion. Access to backed field must be only from property and from constructor. So name of backed field is extra. |
#1551 is a slightly different issue focusing on improving auto-properties rather than traditional properties. |
Why do we need to be able to assign a value to the property scoped field from the constructor? Doesn't that violate the entire point of a property scoped field? |
@GeirGrusom Sometimes we want to initialize property with some value, but as said @bondsbw writing something like |
@KalitaAlexey @bondsbw There is no need for syntax to except constructors from normal scoping rules. It isn't a useful feature, and it introduces typing constructs on properties. Syntax like My opinion is: just let properties define variables inside their own scope. This feature alone is useful in my opinion. Don't overengineer it. |
@GeirGrusom I've thought about any useful case of setting field in constructor without setter and I failed. Indeed if I need change some variable without invoking setter, then my architecture is weak and wrong. |
Good points. I agree that property scoped fields are worth it even without any direct backing field access from the constructor, whose use case is likely small. If we did want to handle that case, however, another way to implement it is in reverse. Instead of the constructor having direct access to the backing field, the property can be told that it is being invoked from the constructor. Perhaps something related to CallerMemberNameAttribute can be used... but I don't have any specific ideas on the matter. All in all, I'd rather have property scoped fields without the ability to distinguish constructor access from others, than to not have them at all. |
This creates a new form of scope which seems like it would be inherently problematic: public class Foo {
private int _value;
public int Value {
private int _value; // legal?
get {
int _value = this._value; // which _value? legal?
return _value;
}
}
} As for consideration number 2, |
Probably not. I can think of few arguments for it and many against.
I'm not a huge fan of that one anyway. Explicit fields give you more power and don't require new keywords. |
Some suggestions, class C
{
public int P1
{
int field;
var initialized = false; // (1) multiple, (2) inferred
get { }
set { }
}
public int P2
{
int field; // (3) same name (well, definitely)
readonly cache = new Dictionary< ... >(); // (4) readonly, inferred
static let static_local = ...; // (5) accessible to both getter and setter
get { }
set { }
}
} and (6) Access modifiers should not be permitted. |
@alrz Did you leave out
|
From a semantic point of view, I would equate the "local" field to a local variable or parameter. So, But then you couldn't have local variables or parameters with the same name. |
If I could go back in time and if I had my way, I would remove the ability to create fields and local variables/parameters with the same name. I can't think of a single time it helped anything, and "this." is a code smell IMHO. I certainly don't want to add to the confusion by allowing property-scoped fields to share names with class fields or local variables. I vote to restrict that capability. |
@bondsbw (1) If you use same convention for parameters and fields e.g. camel case, then this would be helpful. The language should not restrict you on this matter. (2) That's actually a good property of blocks and roslyn itself used it in its codebase alot. In my example above, |
@HaloFour Ah, I see. Never used that target before (only return, and assembly oc). Thanks! |
What do you think about following syntax for quickly defining a backing field? public string FirstName {
get _firstName = ""; // _firstName is 'private string _firstName = "";'
set { _firstName = value ?? ""; }
} Scope of the field is property scoped, of course. Declaration of the field name could be either after the getter or setter. [This post is also in #8364] |
@lachbaer That syntax doesn't seem intuitive, and if my interpretation is correct it would limit property scoping to a single field. I'd prefer public string FirstName {
var _firstName = "";
get => _firstName;
set => _firstName = value ?? "";
} or, without something like #7881, public string FirstName {
var _firstName = "";
get { return _firstName; }
set { _firstName = value ?? ""; }
} |
As you can see you have 3 times written '_firstName'. I want to find a way to avoid that for lightweight properties, as in cases above. |
@lachbaer, what if already have a |
@lachbaer I understand that you're looking to make it more concise, which is a goal I prefer as well. But I don't prioritize conciseness over familiarity and consistency with the rest of the language. If there is only going to be one field allowed, then it should probably just be a keyword. Then you could possibly do something like public string FirstName { get; set => field = value ?? ""; } = ""; where public string FirstName { get; set; } = ""; Another shorthand possibility would be like what you mentioned in #8364, where the setter can be written to return an expression which sets the backing field: public string FirstName { get; set => value ?? ""; } = ""; That just does the same as the first example, but honestly I'm not sure how much real-world use that has other than the null coalescing in this example. Property change notifications, which are a big reason people write setters, need to occur after the setting of the backing field since synchronous event handlers would otherwise read the wrong (old) value of the property. Anyway, the use of a single unnamed backing field goes beyond the scope of this issue, and perhaps should be discussed elsewhere. It isn't necessarily incompatible with property-scoped fields, and I can see the value when it is applicable. |
@paulomorgado No problem. If this proposal comes through the name will be property scoped, otherwise class scoped. @bondsbw The following I have alread proposed in #8364 public string FirstName { get; set => field = value ?? ""; } = ""; Where field is like value a compiler supplied parameter. Just the initialization cannot be done (custom setters cannot be initialized anyway). I only put the |
An alternative might be #12361. One block indention more, but more flexible. |
In terms of #12361 a property would look like local uint _age;
public uint Age {
get {
local _age;
return _age;
}
set {
local _age;
_age = value;
}
} Not very elegant. So in terms of this proposal #850 it could be made: local uint _age;
public uint Age {
local _age;
get => _age: // #7881 syntax
set => _age = value;
} It is just one line of code more, but more consistent if contemplating on #12361 (wich goes a bit farther). |
Assuming that property blocks maintain their own scopes I think the following will be very common, string P1
{
string field;
get => field;
set => Set(ref field, value);
}
string P2
{
string field;
get => field;
set => Set(ref field, value);
} Which certainly is not dry. This is the main use case discussed in the OP where we just need a single field per property. I think something similar to #7614 or #8364 is a more elegant way of doing this. string P1
{
get;
set => Set(ref field, value);
}
Having said that, I think this would be superior to property-scoped fields because properties rarely require more than one backing field. EDIT: However, it will come in handy when you don't have a setter. public object Property => ComputeValue();
public replace object Property
{
object field;
get => field ?? (field = original);
} |
@alrz I don't think it has to be an either-or scenario. I mentioned the |
I love the public string Phone { get => field.Formatted(); set => value.OnlyNumbers(); }
public string Lazy { get => field ?? (field = ReadValue()); }
public string Lazy => field ?? (field = ReadValue());
public int Counter { get; set => field + value; }
public string Log { get { Foo(field); return field; } set { Bar(value); return field = value; }
public string IfDifferent { get; set { if (field != value) { Foo(); } return field = value; } |
There are some issues with an ever existing
Though, I absolutely like the idea of not having a seperate backing field. For the sake of this topic (#850) we should continue the discussion about For this proposal, I think the 'cleanest' solution really is this: class C {
public string Prop {
// property local declarations must come first, like locals in methods (no 'hoisting')
int _field; // no access modifiers allowed
get => _field.ToString();
set => Int32.TryParse(value, out _field);
}
} This would allow for many, many real world scenarios without too much overhead. Internally this could be nicely converted to: class C {
private int <Prop>k__Field__field; // <{PropertyName}>k__Field_{identifier}
public string Prop {
get {
ref int _field = ref this.<Prop>k__Field__field;
{
return _field.ToString();
}
}
set {
ref int _field = ref this.<Prop>k__Field__field;
{
Int32.TryParse(value, out _field);
}
}
}
} |
Why the What's wrong with just:
? |
@paulomorgado |
@lachbaer Auto-properties directly access the backing field without the need for a local ref variable. |
@paulomorgado I think we should leave this issue/proposal with the intend of introducing property local fields. They don't conflict with #8364. |
@CyrusNajmabadi string Name {
ref string field = ref <Name>k__backingField; // as virtual local
get => field;
set => field = value;
} I think that it would be best to have an additional PropertyScopeBinder where this could be placed? |
Issue moved to dotnet/csharplang #626 via ZenHub |
Moved the discussion to csharplang repo. |
Here is a typical C# property that wraps a backing field, such that the property includes a little bit of additional behavior:
Whenever I set
MyProperty
from within that class, I set the property instead of the field so that I ensure the extra behavior runs. This is always the intended mechanism for writing this data.Problem
During maintenance, developers make changes to the class and sometimes manipulate the field directly, instead of setting the property as the class designer intended. The result is that the additional behavior is not run, which leads to subtle bugs. It is a simple mistake, one I would like to prevent if possible.
Solution
One way to fix this situation is to be able to scope fields to their associated property. Within the property, the field can be fully manipulated as expected. Outside of the property, the field is invisible. Methods within that class are only able to interact with the property, which fully encapsulates the field.
Here is an example of what this might look like:
In this case, myField is fully encapsulated within MyProperty and nothing outside of MyProperty has access to myField directly.
Another option
Instead of explicitly declaring the field, perhaps a keyword provides access to an implicit field identified by a keyword, such as
field
. This could look like the following:Additional Considerations
Fields should probably be initialized in a way that bypasses the property setter, such as the following example. The field should be initialized to null, but any future sets must have a non-null value:
Should constructors have special access to the backing field, to be able to set the field directly in the same manner? Dot notation cannot work here. Something different would be necessary, such as
Can the property encapsulate multiple fields? This might be useful in some contexts. It would remove the second option that uses the
field
keyword.The text was updated successfully, but these errors were encountered: