Powershell: Use Nullable<T>.GetValueOrDefault()

[iSazonov]

We have over 270 uses where Nullable.HasValue is before Nullable.Value.
Most of these use cases could be replaced with Nullable.GetValueOrDefault() in some way.
(It is not LINQ!)

https://github.com/dotnet/runtime/issues/33792

[georgettica]

can I give this a go?

[iSazonov]

@georgettica Yes, feel free to pull PR. No need to fix all at one PR - please keep PR as small as possible. I suggest you make only changes of the same type in one PR - there are many different patterns and I suggest to fix only one pattern per PR.

[georgettica]

wow! good job yall! I am gonna back off from the other PR's i've made and let you do your thing

[georgettica]

thanks for all your support @iSazonov

and sry if I made you do double work

[daxian-dbw]

@iSazonov Please see my questions left in #13804 and #13793. It's not clear why changing in thsoe two places.

[daxian-dbw]

Quote my comment from https://github.com/PowerShell/PowerShell/pull/13804#discussion_r524868056:

I think the comment https://github.com/dotnet/runtime/issues/33792#issuecomment-649665286 in that issue has a point. .Value is semantically more correct given that HasValue is already checked. There is a duplicate check on the boolean hasvalue field, but I don't think that affects the perf in practice.

For places where we can use GetValueOrDefault to replace both .HasValue and .Value, it makes more sense to make the change, like in #13805 and #13808. But if we need to keep the .HasValue check, then I think using .Value afterwards is better.

[iSazonov]

I agree in common. Only there is another side - using different patterns decreases readability. I would prefer to use one pattern even though it looks unusual, especially since there are only a few of them in our code base. There is another reason why I prefer this. Now we often look at a code in .Net Runtime for a better understanding of how an application works and we consider this code as the best practice and high quality. Switching to a different code with a different style immediately causes discomfort. I believe that following .Net in this sense is the right direction - many of those who learn PowerShell code will be grateful to us.

[vexx32]

If we're going to use GetValueOrDefault() there, we shouldn't _also_ be checking HasValue. It's fine to use one or the other, but using both is duplicating the same checks unnecessarily.

[daxian-dbw]

Only there is another side - using different patterns decreases readability.

This may be true to some syntax, but to Nullable<T>.Value and Nullable<T>.GetValueOrDefault(), I don't think readability would be affected given that the property/method names are so readable.

[iSazonov]

@vexx32 This could change execution logic - no value and default value can have different semantics in a code path.

[iSazonov]

This may be true to some syntax

Yes, I say about common rule we could/should follow but there are always exceptions to rule. :-)

[daxian-dbw]

When the .HasValue check is necesary, I think we should keep using .Value afterwards. It's semantically more clear that "I want the current value, not the default value". When the .HasValue is unnecessary, it makes sense to use .GetValueOrDefault(), like in #13805 and #13808.

@iSazonov If you agree, I think we should revert #13804 and #13793

[iSazonov]

I have no objection.