Skip to content

Update semi auto props spec around readonly backing field for init-only#5582

Merged
333fred merged 2 commits intodotnet:mainfrom
Youssef1313:patch-6
Dec 27, 2021
Merged

Update semi auto props spec around readonly backing field for init-only#5582
333fred merged 2 commits intodotnet:mainfrom
Youssef1313:patch-6

Conversation

@Youssef1313
Copy link
Copy Markdown
Member

@Youssef1313 Youssef1313 requested a review from a team as a code owner December 27, 2021 20:16
- If the auto-property has no set accessor, the backing field is considered `readonly` ([Readonly fields](classes.md#readonly-fields)). Just like a `readonly` field, a getter-only auto-property can also be assigned to in the body of a constructor of the enclosing class. Such an assignment assigns directly to the readonly backing field of the property.
+ If the auto-property has semicolon-only get accessor without a set accessor, the backing field is considered `readonly` ([Readonly fields](classes.md#readonly-fields)). Just like a `readonly` field, a getter-only auto property can also be assigned to in the body of a constructor of the enclosing class. Such an assignment assigns directly to the backing field of the property.
+ If the auto-property has semicolon-only get accessor (without a set accessor or with an init-only accessor), the backing field is considered `readonly` ([Readonly fields](classes.md#readonly-fields)). Just like a `readonly` field, a getter-only auto property can also be assigned to in the body of a constructor of the enclosing class. Such an assignment assigns directly to the backing field of the property.
```
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol. i got so confused fro a second. i was going to say these two seem almost identical and should be merged. then i realized i was looking at a diff 🗡️

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah reviewing a diff in a diff hurts eyes 😄

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hrmm... does teh constructor actually assign to the field? it doesn't go through the init-setter?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CyrusNajmabadi I think the spec is now wrong regarding this point. I focused more on the first sentence. We need to adjust.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, i think the second sentence needs to be clear that it's only covering the {get;} case and not the init case.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it seems like it's accurate already? It mentions getter-only auto property, so it doesn't apply for the "with an init-only accessor" part of the previous sentence.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right. i think the language is correct, but potentially confusing in context. i wasn't sure if getter only meant: only { get; }, or also somehow applied to get+init (since hte prior sentence covers that). I feel like it could use with jsut a little clarity, only to avoid confusion.

Co-authored-by: CyrusNajmabadi <cyrus.najmabadi@gmail.com>
@333fred 333fred merged commit a3fda95 into dotnet:main Dec 27, 2021
@Youssef1313 Youssef1313 deleted the patch-6 branch December 27, 2021 20:40
333fred added a commit to 333fred/csharplang that referenced this pull request Jan 4, 2022
…dates

* upstream/main:
  Add example of when field initializers will not be run for option 3.
  Correct link
  Added LDM notes for January 3rd, 2022.
  Update README.md
  Update LDM agenda
  Fix line wrapping (dotnet#5589)
  Update raw-string-literal.md
  'record struct' constructor requires 'this' initializer that calls primary constructor or explicit constructor (dotnet#5562)
  Tweak readonly backing field wording (dotnet#5583)
  Update semi auto props spec around readonly backing field for init-only (dotnet#5582)
  Spec change around backing field being readonly (dotnet#5575)
333fred added a commit to Youssef1313/csharplang that referenced this pull request Jan 12, 2022
* upstream/main:
  Update LDM agenda
  Added LDM Notes for January 5th, 2022.
  Update the required members spec (dotnet#5566)
  Add example of when field initializers will not be run for option 3.
  Correct link
  Added LDM notes for January 3rd, 2022.
  Update README.md
  Update LDM agenda
  Fix line wrapping (dotnet#5589)
  Update raw-string-literal.md
  'record struct' constructor requires 'this' initializer that calls primary constructor or explicit constructor (dotnet#5562)
  Tweak readonly backing field wording (dotnet#5583)
  Update semi auto props spec around readonly backing field for init-only (dotnet#5582)
  Spec change around backing field being readonly (dotnet#5575)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants