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: read-only setter (again) #13048

Closed
lachbaer opened this issue Aug 9, 2016 · 12 comments
Closed

Proposal: read-only setter (again) #13048

lachbaer opened this issue Aug 9, 2016 · 12 comments

Comments

@lachbaer
Copy link
Contributor

lachbaer commented Aug 9, 2016

[Other topics are already closed}

In my eyes a validation check for a read-only property value belongs to the property, thus the setter.

I suggest the readonly modifier for the setter:

class Person {
  private readonly string _name;
  public readonly string Name {    // readonly property allows calling setter in ctor only
    get { return _name; }
    set { _name = value ?? "(noname)"; }  // has write access to readonly _name
  }
  public Person(string Name) { this.Name = Name; }
  public Person() : this(null) { }

  static void CreatePersons() {
    var personBob = new Person("Bob");
    var personNoname = new Person(null);
    var personAlice = new Person() { Name = "Alice" };  // Error, setter called after ctor
  }
}

As the setter is readonly

  • it has access to the read-only field
  • it can only be called once in the constructor and never after
  • the compiler can streamline the setter code directly in the appropriate place in the constructor

So the constraint check is in the place where it belongs and all requirements for read-onlys are met.

@svick
Copy link
Contributor

svick commented Aug 9, 2016

var personAlice = new Person() { Name = "Alice" };

How would this work? Object initializer is (mostly) just syntax sugar for calling the setter after the constructor returns.

@lachbaer
Copy link
Contributor Author

lachbaer commented Aug 9, 2016

@svick You're right, my bad. Optional properties should better be initialized by an appropriate design pattern (e.g. builder) in this case. But for constructors it still stands.

@HaloFour
Copy link

@lachbaer

Such a thing cannot be a property setter. No method can update a readonly field, not even if being called from the constructor. The only way what you describe could be implemented would be if the "setter" is inlined by the compiler into the constructor and there would simply be no setter.

I don't see much point to the idea. This validation belongs to the object creation, not to any property assignment.

@bradphelan
Copy link

bradphelan commented Aug 10, 2016

I think the general pattern you are alluding to could be
covered by a keyword once. This means to the compiler
that the method is only ever allowed to be called once per
instance that it belongs to. It is a generalization of
readonly and would cover the case you are suggesting.

public class Foo
{
    public once int Count;
    public once InitCount(int count){
        Count = count;
    }
    public Foo(){
        InitCount();
    }
}

The compiler can statically verify that InitCount will only
be called once and therefore is allowed to initialize
the variable Count once.

Such methods/property setters would only be allowed to be
called from the constructor or from other methods labeled as
once

A potential problem would be

class Foo {
  once void A(){ }
  once void B(){ A(); }
  once void C(){ A(); }

  public Foo(){
    B();
    C();
  }
}

but the compiler could probably detect this case and error
out.

@lachbaer
Copy link
Contributor Author

@HaloFour

The only way what you describe could be implemented would be if the "setter" is inlined by the compiler into the constructor and there would simply be no setter.

That is what I meant by streamlining the code into the constructor.

In my eyes, the verification code does totally belong to the property itself instead of being written in the constructor. With one exception: complex interaction with other read-only fields. But that'll be rather rare, doesn't it? In most cases it will probably be something like a range validation or so, setting reasonable values or throwing exceptions when violated.

BTW, when talking about evacuating the code in setters it should also be possible to attach readonly to methods as well. They will also be callable from the constructor only and compiled in it directly. Of course IntelliSense respects the callability.

@bradphelan I get your idea, but cannot currently think of any frequently used scenarios, except for properties in a builder like fashion.

@bradphelan
Copy link

@lachbaer If you are talking about attaching readonly to methods then we are
talking about exactly the same thing. The disagreement is just over the name
readonly vs once or some other name. I'm not too fussed but I like your
idea.

@dsaf
Copy link

dsaf commented Aug 10, 2016

In my eyes, the verification code does totally belong to the property itself instead of being written in the constructor.

You are right, in a way. Properties are ad-hoc inline classes (state + behavior). What you really want is to introduce a proper one:

class Name
{
    public Name(string value)
    {
        Value = value ?? "(noname)";
    }

    public string Value { get; }
}

Ideally this:

class Name : string
{
    public Name(string value) : base("(noname)")
    {
    }
}

@lachbaer
Copy link
Contributor Author

lachbaer commented Aug 10, 2016

@dsaf Yes, that's a way to achieve it. But with significantly more code, also cluttering the namespace with otherwise useless classes or structs (for value types). Also there will be some more code around to make this work (public readonly Name FirstName = new Name("Alice");) I think that's ugly!

And also that gets me back to my proposal #8364.

public readonly string Name
{
    get;
    readonly set { field = value ?? "(noname)"; } // internally set_Name(string value, ref string field)
}

That's much leaner I think!

readonly in the declaration gives a hint (e.g. if code is collapsed) that this is a readonly property, because the ... { get; } will most likely be not on the same line. I.e. the readonly keyword will be valid for the established readonly properties as well.

readonly set upgrades the set-Keyword to emit a streamline code in the constructor like
this.Name = [ bound content of the setter ]

@dsaf
Copy link

dsaf commented Aug 10, 2016

But with significantly more code...

In practice you are likely to benefit from re-using in medium/large size projects.

Also there will be some more code around to make this work (public readonly Name FirstName = new Name("Alice");) I think that's ugly!

Implicit conversion operator can be implemented if desired.

@lachbaer
Copy link
Contributor Author

@dsaf

In practice you are likely to benefit from re-using in medium/large size projects.

Sure, but I'm talking about small projects or prototyping.

@jnm2
Copy link
Contributor

jnm2 commented Aug 12, 2016

I do what @dsaf suggested but with a struct rather than a class, sacrificing control of default(Name) for better memory and perf. I swear by custom types like this for all domain logic. It's paid off.

@gafter
Copy link
Member

gafter commented Mar 25, 2017

We are now taking language feature discussion in other repositories:

Features that are under active design or development, or which are "championed" by someone on the language design team, have already been moved either as issues or as checked-in design documents. For example, the proposal in this repo "Proposal: Partial interface implementation a.k.a. Traits" (issue 16139 and a few other issues that request the same thing) are now tracked by the language team at issue 52 in https://github.com/dotnet/csharplang/issues, and there is a draft spec at https://github.com/dotnet/csharplang/blob/master/proposals/default-interface-methods.md and further discussion at issue 288 in https://github.com/dotnet/csharplang/issues. Prototyping of the compiler portion of language features is still tracked here; see, for example, https://github.com/dotnet/roslyn/tree/features/DefaultInterfaceImplementation and issue 17952.

In order to facilitate that transition, we have started closing language design discussions from the roslyn repo with a note briefly explaining why. When we are aware of an existing discussion for the feature already in the new repo, we are adding a link to that. But we're not adding new issues to the new repos for existing discussions in this repo that the language design team does not currently envision taking on. Our intent is to eventually close the language design issues in the Roslyn repo and encourage discussion in one of the new repos instead.

Our intent is not to shut down discussion on language design - you can still continue discussion on the closed issues if you want - but rather we would like to encourage people to move discussion to where we are more likely to be paying attention (the new repo), or to abandon discussions that are no longer of interest to you.

If you happen to notice that one of the closed issues has a relevant issue in the new repo, and we have not added a link to the new issue, we would appreciate you providing a link from the old to the new discussion. That way people who are still interested in the discussion can start paying attention to the new issue.

Also, we'd welcome any ideas you might have on how we could better manage the transition. Comments and discussion about closing and/or moving issues should be directed to #18002. Comments and discussion about this issue can take place here or on an issue in the relevant repo.


I am not moving this particular issue because I don't have confidence that the LDM would likely consider doing this.

@gafter gafter closed this as completed Mar 25, 2017
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

7 participants