Skip to content
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 initializer for readonly properties #9516

Closed
alrz opened this issue Mar 5, 2016 · 7 comments
Closed

Proposal: Property initializer for readonly properties #9516

alrz opened this issue Mar 5, 2016 · 7 comments

Comments

@alrz
Copy link
Contributor

alrz commented Mar 5, 2016

Consider the following attribute.

[Foo("Name", Optional = true)]

Currently, for declaring this attribute you may write something like this:

sealed class FooAttribute : Attribute {   
  public bool Optional { get; set; }
  public string Name { get; }
  public FooAttribute(string Name) {
    this.Name = Name;
  }
}

Because you wanted Optional property be, well, optional, you are forced to make it mutable.

Instead, you could write it like this:

sealed class FooAttribute : Attribute {   
  public bool Optional { get; }
  public string Name { get; }
  public FooAttribute(string Name, bool Optional = false) {
    this.Name = Name;
    this.Optional = Optional;
  }
}

But ...

[Foo("Name", Optional: true)] // Optional arguments' names ...
[Foo("Name", true)]           // are optional!

And things can get weirder,

sealed class FooAttribute : Attribute {   
  public bool Optional { get; set; }
  public string Name { get; }
  public FooAttribute(string Name, bool Optional = false) {
    this.Name = Name;
    this.Optional = Optional;
  }
}

[Foo("Name", Optional : true)] // Optional = true in ctor
[Foo("Name", Optional = true)] // Optional = false in ctor and it is assigned twice!

Actually, it might be unfortunate that despite of the fact that you are assigning Optional inside of the constructor argument list, it gets evaluated after that, just like property initializers, unless you will know that because property initializers are enclosed in { } separate from the constructor.

The idea is to allow such immutable properties to be initialized by name inside of the constructor parameter list, optionally; and this would not be just for attributes — making the syntax more consistent, meaningful and widely usable.

sealed class FooAttribute : Attribute {   
  public bool Optional { get; } = false;
  public string Name { get; }

  public FooAttribute(string Name, ...) {
    this.Name = Name;
  }
}

[Foo("Name")]                  
[Foo("Name", Optional = true)] 

new FooAttribute("Name")
new FooAttribute("Name", Optional = true)

Under this proposal, ... generates optional arguments for each property that has an initializer.

public FooAttribute(string Name, bool Optional = false) {
  this.Optional = Optional;
  this.Name = Name;
}

In fact, it doesn't matter that what value it is initialized with, because the assignment is emitted in the constructor body anyway.

// normally
public FooAttribute(string Name) {
  this.Optional = false;
  this.Name = Name;
}

We just used it for default value of the corresponding parameter. The important thing is, in case of a reference type it will be translated to a null default and a null coalescing operator.

sealed class FooAttribute : Attribute {   
  public object Optional { get; } = new object();
  public string Name { get; }

  public FooAttribute(string Name, ...) {
    this.Name = Name;
  }
  // translates to
  public FooAttribute(string Name, object Optional = null) {
    this.Optional = Optional ?? new object();
    this.Name = Name;
  }
}

Because these properties should have a default value, using #7737 and #206 you will have,

sealed class FooAttribute(string Name, ...) : Attribute {   
  public bool Optional { get; } = default;
}

And additional constructor parameters will be generated along with the record expansion.

Related: #229

@HaloFour
Copy link

HaloFour commented Mar 6, 2016

Your implementation would make it impossible to explicitly pass a null to an optional argument of a reference type. While I'm sure that you're using attributes for illustration it is worth nothing that attributes play by a completely different set of rules than normal object instantiation.

@HaloFour
Copy link

HaloFour commented Mar 6, 2016

I also don't see any issue in optional constructor argument names actually being optional. If I want to save a few keystrokes and rely on positional construction (which is, ya know, the default), why shouldn't I be able to?

@alrz
Copy link
Contributor Author

alrz commented Mar 6, 2016

@HaloFour

Your implementation would make it impossible to explicitly pass a null to an optional argument of a reference type.

class Foo {
  public object Optional { get; } = null;
  public Foo(...) {}
  // translates to
  public Foo(object Optional = null) {
    this.Optional = Optional ?? null;
    // however, it can be optimized to the following because the default is null
    this.Optional = Optional;
}
// and then
new Foo(Optional = null)

it is worth nothing that attributes play by a completely different set of rules than normal object instantiation.

They already do, Optional = value inside constructor argument list turns out to be a property initializer which in turn, requires the property being publicly mutable.

I also don't see any issue in optional constructor argument names actually being optional.

So why would you use named arguments in attributes in the first place, you could define all of them in the constructor (perhaps, as optional arguments), right?

@HaloFour
Copy link

HaloFour commented Mar 6, 2016

@alrz

I meant for the consumer of said attribute. If the default was new object() but you wanted to pass null, you'd be SOL because the code couldn't tell from an explicit null v. an omitted argument.

If object initialization syntax was considered in the C# 1.0 timeframe it's possible that we would have seen different syntax for attributes. Object initialization does require mutable properties, as does attribute initialization via properties. Said initialization has its advantages over default parameter values given that the latter requires that the value be embedded at the call site and you can't really detect an omitted value vs. a specified default value.

@alrz
Copy link
Contributor Author

alrz commented Mar 6, 2016

@HaloFour If a null is desired, the default should have been null too because immutable properties are not meant to be changed. perhaps you should consider a full properties declaration for that matter, e.g.

get { if(field == null) { field = new object(); } return field; }

It doesn't make sense to provide a syntax for every possible situation.

Object initialization does require mutable properties

So you're against #229 too, right? because the point is to initialize readonly auto properties without an explicit constructor declaration with parameters and initializations.

@HaloFour
Copy link

HaloFour commented Mar 6, 2016

@alrz

If a null is desired, the default should have been null too because immutable properties are not meant to be changed.

No, if an explicit null is desired the "optional" argument shouldn't decide to override it with whatever the default happens to be. This is not the behavior for either property initializers or optional parameters:

string TestOptional(string value = "default") => value;
...
Debug.Assert(TestOptional(null) == null);

So you're against #229 too, right? because the point is to initialize readonly auto properties without an explicit constructor declaration with parameters and initializations.

I find it unnecessary. I don't think I'm the only one: #9330 (comment)

@alrz
Copy link
Contributor Author

alrz commented Mar 6, 2016

@HaloFour In #9330 it has been proposed to use case-insensitive name comparisons in property initializers, it doesn't suggest anything to actually avoid the constructor.

Anyway, I think this scenario has got enough attention and probably it is no longer an issue.

@alrz alrz closed this as completed Mar 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants
@alrz @gafter @HaloFour and others