Skip to content

add ref local reassignment #7

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

Merged
merged 4 commits into from
Apr 11, 2018

Conversation

BillWagner
Copy link
Member

And build project around the sample.

Sample updates for dotnet/docs#3961

@BillWagner BillWagner requested a review from rpetrusha April 3, 2018 22:32
@BillWagner BillWagner changed the title [WIP] add ref local reassignment add ref local reassignment Apr 4, 2018
{
retval += $"{numbers[ctr]} ";
}
return retval.Trim();
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that is pre-existing code, but to make snippet shorter what about such ToString() implementation:

public override string ToString() => string.Join(" ", numbers);

Same remark also for the NumberStoreUpdated.cs file

And build project around the sample.
The newer version shows a sample where the ref local reassignment feature creates more efficient code.
Use expression bodied members for ToString.
@BillWagner BillWagner force-pushed the csharp73-ref-local-reassignment branch from 7cd9599 to ed9ea0f Compare April 4, 2018 22:40
Copy link

@rpetrusha rpetrusha left a comment

Choose a reason for hiding this comment

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

LGTM, @BillWagner, except I'm wondering what the // #error version comment in Program.cs means. You can merge when you're ready.

@BillWagner BillWagner added the 🚧 Hold for related PR Indicates a PR can only be merged when other related PRs are merged (see comments for links) label Apr 10, 2018
@BillWagner
Copy link
Member Author

This PR must not be merged until dotnet/docs#4875

@BillWagner
Copy link
Member Author

Thanks @rpetrusha That comment was a test and a reminder to add a note on another issue.

@BillWagner BillWagner merged commit bd817d8 into dotnet:master Apr 11, 2018
@BillWagner BillWagner deleted the csharp73-ref-local-reassignment branch April 11, 2018 18:13
@mairaw mairaw added the 📁 Repo - samples Indicates PRs done in the samples repo. label Apr 27, 2018
karelz pushed a commit to karelz/samples that referenced this pull request Aug 31, 2018
* add ref local reassignment

And build project around the sample.

* update code sample

The newer version shows a sample where the ref local reassignment feature creates more efficient code.

* feedback

Use expression bodied members for ToString.

* remove the error pragma comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚧 Hold for related PR Indicates a PR can only be merged when other related PRs are merged (see comments for links) 📁 Repo - samples Indicates PRs done in the samples repo.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants