-
Notifications
You must be signed in to change notification settings - Fork 4k
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: Semi-auto-properties with setters [topic has evolved] #8364
Comments
I like my suggestion more ;-) No, really! :) Think, that it's a neat re-use of the Also I like the idea of Property-scoped fields #850 ! But it stands in no contrast to this issue. |
Really neat ! |
Addition: ...
value _backingField;
... should only be present 1 time, either after public string HelloMessage
{
get value _backing_ONE;
private set value _backing_TWO; // WTF, why 'two'?
} There might be cases where you want this intentionally, but I recommend using the good old Having public string HelloMessage
{
get value _backingField;
set {
if (value == null) throw new ArgumentNullException();
set value; // tells compiler to write to assign to declared _backingField
}
} Please notice the |
The lambda ( private int x;
public void set_X(int value) => this.x = value; The I really don't understand the point of specifying the backing fields at the scope of the accessors. This proposal just seems like a syntactic jumble. I certainly prefer the syntax proposed in #850, at least where fields can be declared within the scope of the property. |
@HaloFour you probably got me wrong. The
For Maybe a single public string AnotherMessage {
get; // implicitly created getter, we don't have access to the backing field!
// unless of course no 'value ...' instruction is given before 'get;'
// This short 'get;' allone tells the compiler to create an auto-property.
set {
TestForNullValue(); // e.g.
set; // put the value in the backing field, though we don't know it.
}
} = "Hello again";
I also agree using the So, after all for me it get's down to public string HelloMessage { value _customHelloMessage; get; private set; } = "Hello";
// or having 'get' and 'set' implicity implemented when ommited:
public string HelloMessage { value _customHelloMessage; } = "Hello";
public string AnotherMessage { get; set { doSomething(); set; } } dropping off all of this Should I open a new issue for this revised idea? |
I think it's fine for a proposal to evolve in place. In effect it is a declaration if the result is an instance field. But I don't see a real use for having it if you don't intend to use it directly as an identifier. I'm personally not a big fan of new syntax that straddles normal properties and auto-properties. I think it confuses the reason as to why you'd use one or the other. But let's break this down into the use cases. Namely, it's about being able to add custom validation or logic to the accessor, most likely the setter accessor. Currently that requires going from: public string Foo { get; set; } to private string foo;
public string Foo {
get { return foo; }
set {
if (value == null) throw new ArgumentNullException("value");
foo = value;
}
} That is big jump in verbosity and it is understandable to want to find some kind of short-hand. Perhaps something like the following: public string Foo {
get;
set {
if (value == null) throw new ArgumentNullException("value");
set;
}
} Although does that honestly buy you much? You eliminate the getter boilerplate, the instance field declaration and the manual assignment, but the property is still almost as long. And is it really clear what the property is going to do? I'm not completely sold, but I can see value in going down that path. As for syntax for declaring the backing field, I don't think that has any value unless you actually use that field directly. I don't see proposals like #850 as being about verbosity but rather about visibility; trying to prevent accidental direct assignment of the backing field. I wouldn't expect a field declared within the body of a property to be accessible anywhere else in that class. So the two examples where you define the backing field and never refer to it directly in the accessors has no value to me. |
How about just:
Or more extreme:
|
Extreme is right. Definitely on the wrong side of the succinct/terse spectrum in my opinion. Do we really need a syntax shorter than auto-properties? What is |
@HaloFour: the backing field (in that example) |
Declared elsewhere or entirely implicit? If the latter what is its visibility? Type? |
@HaloFour backing field was a bit wrong terminology :D I meant just any field (but you one you use to back the property with) Updated example |
@leppie I think that the term is appropriate, it just didn't clear up the confusion I had as to where it was declared. 😄 I think that it's a pretty far departure to property syntax today. I think that I'd prefer something more akin to: private string bar;
public string Foo { get => bar; set => bar = value; } Which is effectively just normal properties combined with expression members. |
In your last sample you wrote 3 times 'bar'. This is slightly shorter, especially if you want many properties with own backing variables: private string bar1;
private string bar2;
private string bar3;
//...
private string bar19;
private string bar20;
public string Foo1 { value bar1; }
public string Foo2 { value bar2; }
public string Foo3 { value bar3; }
// ...
public string Foo19 { value bar3; /* uses field bar3 for some reason */ }
public string Foo20 { value bar20; } One could also use private int _counter;
public int Counter {
value _counter;
get {
value = CalculateSomething(value);
return value;
}
private set;
} When you change your mind and you want to switch from I think it even gets more intersting when we reference other properties instead of fields with the public string UnfancyProp { get; set; }
public string FancyProp {
value UnfancyProp;
get;
set => "Fance me up " + value; Writing this brings me back to the lambda operator public string AutoProp { // "standard, old fashioned" auto-property
get;
set => "Hello " + value;
} If one doesn't want to set a value to the backing or wants to do some other important stuff, she should use the common |
Why would you ever want to use that syntax? It makes no sense to declare a backing field and a property that does nothing more than wraps that backing field. We already have auto-properties. Using I don't see the use cases here. |
You're giving me headaches :-D ;-) public string Foo {
get;
set => { if (value==null) throw new ArgumentNullException(); return value; }
} here we have an auto property where we are not really interested in the backing field, just in the argument checking. Without auto-property it is slightly more cumbersome to write. Let's try another syntax, still c-sharpy, because the property kind of 'inherits' the backing field: private string _backingField;
public string Foo {
get => "Foo is " + value ; // oops, no more hints for an auto-prop!!!
set => { if (value==null) throw new ArgumentNullException(); return value; }
} : _backingField = "Initial string"; Now it is obvious what private T CheckNull<T> (T myvalue) {
if (myvalue==null) throw new ArgumentNullException();
return myvalue;
}
/* complete auto-property, just with a bit of setter logic */
public string FirstName{ get; set => CheckNull<string>(value) } = "";
/* here no auto-property can be created, because getter and setter are defined,
therefore we reference the backing field */
private string _lastName ;
public string LastName{
get {
Console.WriteLine("Another get on family " + value);
return value;
}
set => CheckNull<string>(value);
} : _lastName = "";
public string FullName { get => FirstName + " " + _lastName ; } And if this is too much code for a sample, here's an auto-property with explicit backing field: private int _bar;
public int Foo { get; set; } : _bar;
public var Foo2 { get; set; } : _bar; // type inference, looks ugly... |
I get what you want. What I don't understand is why. If you're going to the trouble of defining a backing field then you have no reason to use auto-implemented properties. Trying to hammer them together makes little sense and solves no problems. As for trying to add logic into auto-implemented properties, I must note that the charter for auto-implemented properties is to provide a concise syntax when no additional logic is required in the property accessors. I'm largely of the opinion that trying to add logic back into auto-implemented properties doesn't make a lot of sense particularly since it's quite easy to define a full-fledged property. That said I think that there may be some compelling cases to trying to squeak validation into an auto-implemented property setter accessor but only if the syntax is concise enough and clear enough and I haven't seen anything that I think fits the bill. |
A little summary by now:
/* this is still an auto-property */
public int Foo { get; set => (value<0) : 0 : value; }
For self-defined properties (not auto), if you want to use the expression syntax on private int _foo;
public int Foo { get => value; set => (value<0) ? 0 : value; } : _foo; Assigning a backing field manually to an auto-property could be done, but actually makes no sense. But maybe it might be easier to implement it in the complier that to 'explement' it ;-) protected int _foo;
public int Foo { get; protected set; } : _foo; |
@HaloFour private T CheckNull<T> (T myvalue) {
if (myvalue==null) throw new ArgumentNullException();
return myvalue;
}
public string MrMrs { get; set => CheckNull<string>(value) } = "";
public string FirstName{ get; set => CheckNull<string>(value) } = "";
public string MiddleName{ get; set => CheckNull<string>(value) } = "";
public string LastName { get; set => CheckNull<string>(value) } = "";
public string Address1{ get; set => CheckNull<string>(value) } = "";
public string Address2{ get; set => CheckNull<string>(value) } = "";
public string ZipCode{ get; set => CheckNull<string>(value) } = "";
public string City { get; set => CheckNull<string>(value) } = "";
public string Country { get; set => CheckNull<string>(value) } = "";
public int Age { get; set => (value<0) ? 0 : value; }; This was written really fast with copy-paste. Specifying the backing field and using |
Having the setter accessor expression require a returned value doesn't make a lot of sense given that setter accessors return |
@HaloFour |
It's not consistent. To be consistent it should behave as a property setter is supposed to, including having a |
Can you rewrite the above addess sample without |
Not with auto-implemented property syntax given that there is no way to directly access the backing field. You could potentially add a new contextual keyword to reference it but that doesn't seem terribly useful, not to mention confusing if that keyword is in use in any other way. With normal properties, you could potentially do the following: private void CheckNull<T>(T proposed, ref T field) {
if (proposed == null) throw new ArgumentNullException();
field = proposed;
}
private string name;
public string Name { get => name; set => CheckNull(value, ref name); } That said, I figure that expression member syntax would apply more to the getter than the setter. |
I still stick with the basic idea of my proposal. Auto-properties were introduced, because they were shortening code a lot. Expression body methods and properties were introduced with C# 6 to shorten this. Of course this could all be done with the old language features, but what's the idea behind evolving a language? There are already some easements, like Expression Body Definitions [(https://msdn.microsoft.com/en-us/library/x9fsa0sw.aspx#Anchor_0)]. We can go some steps further. You might be right that declaring an external backing field and still using coding shortcuts is nothing that would be done often in real code, probably never seriously. So I drop this idea for the moment. Let's focus on having more control over auto-props, thus reducing the need for an instanciated backing variable: private string _customHelloMessage = null;
public string HelloMessage
{
get { return _customHelloMessage ?? "Hello World"; }
set { _customHelloMessage = value; }
} This is already using new C# features, but nevertheless it is quite long for achieving a small thing. Actually I don't really need the With my idea we could shorten this long code a lot! public string HelloMessage
{
get => value ?? "Hello World";
set;
} = null; Next I want to check for public string HelloMessage
{
get;
set => value ?? "Hello world";
} = null; // initial value goes through setter for integrity! I think we agree that Getting back to the idea of #850, my proposal for the use of the Now there might be scenarios, where you still want some more control over your code, but you also want kind of an auto-prop. This might be the case if your code grows. Here I introduce the new contextual keyword public virtual int Foo
{
let backingField = 0; // creates a backing field with the appropriate type, optionally initializes it.
get;
set => backingField + value;
} = 12; // initializes again, but using the setter this time
// one useless sample to make it clear ;)
public int Counter {
let mycounter = 0;
get => ++mycounter;
set => 0; // any set call resets the counter
}
Derived classes don't have access to the property internal backing field of the base class. It would not be intuitive in the derived classes and mess things up. Résumé: Any occurence of |
Shorter code is not always better code. Clarity is significantly more important. Code will be read (by humans) significantly more often than it will be written, so the language should always optimize for that case. I disagree with the premise of trying to create a spectrum of syntax between normal properties and auto-implemented properties. I don't think that normal properties are so verbose as to be a burden on a developer, particularly when you need additional logic. |
After a night of sleep I have to say that But for semi-auto-properties the signature for internal set
{
var oldValue = field;
OnFooSemiPropChanging(oldValue, /* new */ value);
field = value;
OnFooSemiPropChanged(oldValue, /* new */ value);
}
What still bothers me is, that I'd like to have a the set routine return a value for assignment set => (value > 0) ? value : throw new ArgumentOutOfRangeException(); What about an additional |
The longer I think about it, I have to admit that having a /* "field" is a reference to the underlying synthesized auto-property backing field */
public int Foo {
get;
set { field = (value > 0) ? value : throw new ArgumentOutOfRangeException(); }
/* or */
set => field = (value > 0) ? value : throw new ArgumentOutOfRangeException();
} could be rewritten to public int Foo {
get;
put { return (value > 0) ? value : throw new ArgumentOutOfRangeException(); }
/* or */
put => (value > 0) ? value : throw new ArgumentOutOfRangeException();
} That last set { field = (value > 0) ? value : throw new ArgumentOutOfRangeException(); }
put { return (value > 0) ? value : throw new ArgumentOutOfRangeException(); } Here there is really not much of a gain - if not even none at all... But this: set => field = (value > 0) ? value : throw new ArgumentOutOfRangeException();
put => (value > 0) ? value : throw new ArgumentOutOfRangeException(); Of course the 2nd with The observant reader might have noticed the use of a local called In case of a semi-auto-propery the signature of void set(T value); to void set (T value, ref T field); /* field is a reference to the synthesized backing field */ This would keep syntax changes nearly not recognizable, and so stays in line with the existing C#. But it empowers C# to define setters for lightweight auto-properties, semi-auto-properties. [ADDENDUM] The public signature must not change, because otherwise you cannot convert an auto-property to a semi-auto-property without recompiling all referencing binaries. So, this must be internally converted to private void set (T value, ref T field); /* field is a reference to the synthesized backing field */
public void set(T value) => set(value, ref <Foo>k__BackingField); For private or assembly internal calls the call should go to the extended set method directly. |
@gafter I have implemented that previously described behaviour as a first feature preview in Roslyn and would like to submit a pull request, after I have prepared the branch for it. [UPDATE:] There are still some issues that must be fixed (see addendum above). E.g. now a set_XXX method is generated with the call breaking non-optional ref field. Apart from that it works fine :-) Can anybody shed a light for me on how to achieve the wanted rewriting? Any hints where I could look such things up in the code? |
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 (#850), or class scope as long as #850 is not implemented. Declaration of the field name could be either after the getter or setter, otherwise the property is probably more complex and a shortcut is not needed. Advantages over the previous suggestion:
Disadvantages:
|
In #850 @bondsbw complained that the last syntax is not concise. I agree somehow - it was just a thought. I'll try to point out my goal with this proposal and why it seems important to me. In the beginning the only way to define a lightweight, no intelligence property like this: private string _firstName = "";
public string FirstName {
get { return _firstName; }
set { _firstName = value; }
} You will notice that _firstName is written down 3 whole times, just to achieve to have simple property. Quite a lot of code for light goal... That probably lead to code that relies on public class fields ('variables') more than properties as 'public interface'. It was just too much to write, even with code snippets. Then, probably for that very reason, in C# 3.0 auto-properties were invented. Now the above code shortens to public string FirstName { get; set; }; Much better, but there was no way to initialize it. The first access to the above property returns C# 6 corrects this public string FirstName { get; } = "";
public string LastName { get; } = "(none)";
public string FullName => $"{FirstName} {LastName}";
public int Age { get; set; } = 0; Much better, immutable and with initialization. Now to the reasons of this proposal: So, the reason why auto-properties were invented and extended with further possibilities were in the first place was to encourage programmers to use them to make their code more stable and reliable, preventing weak spots and points of failure in the applications. I want to try to find a syntax that fits into the style of C# and that allows writing of lightweight properties. Those are properties that have no explicitly backing field and only a custom setter or custom getter. [There might be reasons to recalculate the value the moment it is accessed]. The following quote from MSDN should still stay true for semi-auto-properties:
I cannot emphasize enough that this proposal is mainly about verification setters. If it emits a syntax feature that is short, precise and could do more, I'm fine :-) |
As local functions will come with C# 7 very likely the following solution might be the currently best approach. public string FirstName {
get;
set { field = value ?? ""; }
} = "none"; will rewrite to ('k__' = compiler generated symbols) private string k__BackingField = "none";
public string FirstName {
get { return k__BackingField; }
set {
void k__set_value(string value, ref string field) { field = value ?? ""; }
k__set_value(value, ref k__BackingField);
}
} likewise for Rewriting takes place later in the compiler, so this might be a good way? |
One sample for a semi-auto-property with getter: public string FirstName {
get => field ?? "";
set;
} // = null is implicitly initialized This would avoid accidentially initializing the property with Maybe it is a wise idea to make an initialisation mandatory for semi-auto-properties! It ensures that the programmer choses a suitable starting value for the property, that complies with his setter rule, instead on relying on the compilers chosen init value, that might violate it. |
One point, re the above: there is no immutability in what you have declared there. It's a completely mutable set of properties. |
@DavidArno Typo, corrected it in the sample above. Tnx. Any other comments from you on this topic? :-) |
My thoughts on this topic are the same as for your proposal to allow expression bodies for public string FirstName { get; } = "";
public string LastName { get; } = "(none)";
public string FullName => $"{FirstName} {LastName}"; then you should have to write more code, eg by having an explicit backing field in the case of this proposal. I may be in a minority amongst C# users in this regard, but that's my position and I'm sticking to it :) |
I just have another idea in mind, regarding the return value of a setter for semi-auto-properties. public string Name {
get; // indicates (semi-)auto-property
set => value ?? "(none)";
}
What doesn't feel right is that set once expects a return value, once doesn't... |
In accordance to #13048 I like to update my proposal: [Updated post]
public readonly string Name // can only be set in constructor,
{
get;
set return => value ?? "(noname)";
}
|
I made up my mind for quite a time now and my (final) suggestion for semi-auto-properties is as follow. public string Name
{
get;
set { field = value ?? "(empty name"); }
} = "(not set)"; or public string Name
{
get;
set => value ?? "(empty name");
} = "(not set)";
// for block bodies:
void set_XXX(T value)
=> SetXXX(value, ref backingField);
private void SetXXX(T value, ref T field) { ... }
// for expression bodies that return other than void
void set_XXX(T value)
=> backingField = SetXXX(value, ref backingField);
private T SetXXX(T value, ref T field) { ... }
(What should still be resolved is what happens with concurrency) |
In #850 there came up the idea of a I have to update my previous post now, because this ever existing in getter and setter has problems.
Therefore I update my proposal (again ;-) [but it's getting better every time 😄 ] In the following cases, a public T PropertyOne {
get;
set => field = value;
}
public T PropertyTwo {
get => field;
set;
}
public T PropertyThree {
get => field;
set => field = value;
} = default(T); In In In
Under the hood all this could be nicely implemented with the new private T <PropertyThree>k__BackingField;
public T PropertyThree {
get {
ref T field = ref this.<PropertyThree>k__BackingField;
{
return field;
}
}
set {
ref int field = ref this.<PropertyThree>k__BackingField;
{
field = value;
}
}
} = default(T); // initialisation circumvents setter! I think, this is now a quite complete approach. Other opinions? 😄 |
Makes sense. I agree that the PropertyThree case is not necessary, though I also wouldn't mind if it exists for consistency. What happens for cases where |
@bondsbw The |
Discussion moved to dotnet/csharplang#140. |
Closing issue since discussion moved to csharplang. Thanks |
Using a backing field without naming it was considered in https://github.com/dotnet/csharplang/blob/master/meetings/2020/LDM-2020-04-01.md |
Status 2016-03-02
This topic has evolve/changed from providing a name for the backing field to extending the auto properties of C# with setters, thus providing semi-auto-properties.
The initial proposal was droped on that way.
Original topic start
According to an Expression Body Definition of a property with the lambda operator it would be consistent to allow the
=>
operator also in theget
section of a propertyThis would be consistent as the lambda operator introduces a
return
expression.Having this said, it would also be nice to shorten the
set
section of the property. Here I have some ideas.For the following samples I assume that there is a field
private string _customHelloMessage = null;
First we could also use the
=>
operator, but I don't think that is a good idea as set is not areturn
expression. Just for an optical impression:Looks better, but the only thing that makes a real differnce is the letter s or r in
get
andset
. Also, as said, it is not consistent to the current use of=>
.The next idea is to extent the contextual keyword
value
to stand withset
:It looks a bit odd, but should be intuitive.
This idea could be extended for the case that you want to specify the backup field by yourself:
Also note the implicit initialization, that sould be further possible for auto-implemented properties;
The above sample could be abbrevated even more with the
value
keyword declaring the backup field within the property definition:what we could even more shorten with
In case there should be modifiers,
get
andset
must be explicitly stated.The get and set methods can be overwritten, but at least one must be 'default' (
get;
orset;
) to make sense for thevalue
being there, if it's there.A backup field must never be implied automatically by the compiler, meaning the
_customHelloMessage
needs an explicit declaration. Otherwise it will be error-prone.I hope this is a worthy idea.
Regards,
Ike
The text was updated successfully, but these errors were encountered: