Skip to content

Clarify "string" vs "System.String" in C# programming guide #24985

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 9, 2021
Merged

Clarify "string" vs "System.String" in C# programming guide #24985

merged 1 commit into from
Jul 9, 2021

Conversation

aaronfranke
Copy link
Contributor

Summary

string and String are only equivalent if you have using System; in your file. This PR improves the docs to clarify this.

@aaronfranke aaronfranke requested a review from BillWagner as a code owner July 4, 2021 17:53
@dotnet-bot dotnet-bot added this to the July 2021 milestone Jul 4, 2021
@MSDN-WhiteKnight
Copy link
Contributor

therefore string is more portable and is recommended

"Portable" is generally used when talking about different architectures or operating systems, i think it could be confusing here. Using language keywords is recommended not because it enables avoiding using System;, this directive is common anyway; but actually, it is because identifiers can be resolved to different symbols depending on what is imported into current scope. So we could say something like "using string is recommended, because it unambigously refers to the System.String type in any context".

@IEvangelist
Copy link
Member

Hi @aaronfranke thank you very much for your PR, it's much appreciated. With that said, I've provided a review - and wanted to offer up a few thoughts. Looking forward to hearing back from you, thanks.

@IEvangelist IEvangelist added the needs-more-info Needs more info from OP. Auto-closed after 2 weeks if no response. [org][resolution] label Jul 8, 2021
@aaronfranke aaronfranke requested a review from IEvangelist July 8, 2021 16:33
@aaronfranke
Copy link
Contributor Author

I updated the PR with your suggestions and also added a note about using System;.

BillWagner
BillWagner approved these changes Jul 9, 2021
View reviewed changes
Copy link
Member

@BillWagner BillWagner left a comment

Choose a reason for hiding this comment

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

Thanks for all the work and discussion @aaronfranke @IEvangelist @Youssef1313

This LGTM. Let's :shipit:

IEvangelist
IEvangelist approved these changes Jul 9, 2021
View reviewed changes
Copy link
Member

@IEvangelist IEvangelist left a comment

Choose a reason for hiding this comment

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

Looks good to me, we'll :shipit: - thank you 🙏

@IEvangelist IEvangelist merged commit c7ccbe4 into dotnet:main Jul 9, 2021
@aaronfranke aaronfranke deleted the string branch July 9, 2021 14:08
@MSDN-WhiteKnight MSDN-WhiteKnight mentioned this pull request Jul 23, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Youssef1313 Youssef1313 Youssef1313 left review comments

@BillWagner BillWagner BillWagner approved these changes

@IEvangelist IEvangelist IEvangelist approved these changes

Assignees
No one assigned
Labels
dotnet-csharp/svc fundamentals/subsvc lang-reference/subsvc needs-more-info Needs more info from OP. Auto-closed after 2 weeks if no response. [org][resolution]
Projects
None yet
Milestone
July 2021
Development

Successfully merging this pull request may close these issues.
6 participants
@aaronfranke @MSDN-WhiteKnight @IEvangelist @BillWagner @Youssef1313 @dotnet-bot