Skip to content

Nullable .Value investigate and replace with .GetValueOrDefault() #2612

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

Open
TomFinley opened this issue Feb 19, 2019 · 4 comments
Open

Nullable .Value investigate and replace with .GetValueOrDefault() #2612

TomFinley opened this issue Feb 19, 2019 · 4 comments
Labels
code-sanitation Code consistency, maintainability, and best practices, moreso than any public API. P3 Doc bugs, questions, minor issues, etc. perf Performance and Benchmarking related up-for-grabs A good issue to fix if you are trying to contribute to the project

Comments

@TomFinley
Copy link
Contributor

TomFinley commented Feb 19, 2019

In #2579, it was revealed that full nullable values, if you know that the nullable has a value at a certain point that calling GetValueOrDefault() is faster than .Value, despite having equivalent code, since an unnecesssary check is avoided. While obvious in retrospect given the reasoning, that is not what most of our existing code is doing.

Given that insight, in principle we should not be using .Value in any case since if at a certain point we expect that a nullable value should not be null (because we checked, or because we require it to be non-null for some other reason), we would not consider the exception thorn by the .NET framework out of .Value to be sufficiently descriptive and helpful anyway. So the hardest part of this would actually be checking that everywhere we use .Value that we do have the appropriate checks in place, since otherwise it should be a relatively straightforward replacement.

While I don't expect a measurable perf impact, since I don't think we use nullables in tight loops too often if anywhere, it certainly wouldn't hurt and would help to maintain best practices.

Not critical work for v1, since it deals with internal code.

/cc @stephentoub

@TomFinley TomFinley added up-for-grabs A good issue to fix if you are trying to contribute to the project perf Performance and Benchmarking related code-sanitation Code consistency, maintainability, and best practices, moreso than any public API. labels Feb 19, 2019
@lethienhoang
Copy link

Hi, I have been worrking as c# developer since 2016. So I want to contribute to open source but I don;t have experience much. WHo can lead me? Thanks

@veikkoeeva
Copy link
Contributor

In detail reasoning at dotnet/coreclr#22297.

@Akshive
Copy link

Akshive commented Aug 14, 2019

I would like to work on this, but I might need some help because I am new to this.

@jwood803
Copy link
Contributor

jwood803 commented Dec 9, 2019

Unable to close this, but due to what was found in the PR, this may cause more of a performance hit instead of lessening it.

@frank-dong-ms-zz frank-dong-ms-zz added the P3 Doc bugs, questions, minor issues, etc. label Jan 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-sanitation Code consistency, maintainability, and best practices, moreso than any public API. P3 Doc bugs, questions, minor issues, etc. perf Performance and Benchmarking related up-for-grabs A good issue to fix if you are trying to contribute to the project
Projects
None yet
Development

No branches or pull requests

6 participants