Open mattjohnsonpint opened 1 year ago
Thank you for pointing this out.
@tarekgh do you have any thoughts on this?
I'm also going to open an API proposal for this extension method, as I believe it should be built-in.
I'm also going to open an API proposal for this extension method, as I believe it should be built-in.
I think that's the https://github.com/dotnet/runtime repo
Yes, but the docs should be updated in the meantime so users don't introduce bugs. I can send a PR if you agree.
We love PRs :) but @tarekgh should review, they're the subject matter expert.
@mattjohnsonpint what exactly the issue you are trying to help with here? Is it to give the choice for users to decide which time to get when having ambiguous time? and ensure getting some results when having invalid time?
For these doc pages, the main issue is that the example code incorrectly uses BaseUtcOffset
without accounting for BaseUtcOffsetDelta
.
DateTime utcTime = DateTime.SpecifyKind(ambiguousTime - TimeZoneInfo.Local.BaseUtcOffset, DateTimeKind.Utc);
That is the bug in the example. The standard offset is the one that is the lesser of the two ambiguous offsets - but it is not always going to match BaseUtcOffset
.
I'll open a separate issue in the runtime repo for the API proposal.
It also shows TimeZoneInfo.Local
there, which may or may not be applicable.
The other page shows a similar bug, but only in the VB sample.
If offsets(ctr).Equals(TimeZoneInfo.Local.BaseUtcOffset) Then
zoneDescription = TimeZoneInfo.Local.StandardName
Else
zoneDescription = TimeZoneInfo.Local.DaylightName
End If
Again, there's no guarantee the BaseUtcOffset
is the standard offset, because BaseUtcOffsetDelta
may modify it.
Ok, thanks @mattjohnsonpint. submit the pr and I'll be happy to look at it.
In several places in the guidance for resolving ambiguous
DateTime
values (both in this doc and this doc), the example code uses theBaseUtcOffset
property of the time zone to determine the offset for standard time. This unfortunately is a bug, because it does not take into account anyBaseUtcOffsetDelta
that may apply.More about
BaseUtcOffsetDelta
in the blog post here: https://devblogs.microsoft.com/dotnet/date-time-and-time-zone-enhancements-in-net-6/#timezoneinfo-adjustmentrule-improvementsBoth
GetUtcOffset
andGetAmbiguousTimeOffsets
will takeBaseUtcOffset
andBaseUtcOffsetDelta
into account - the problem is only when usingBaseUtcOffset
by itself, because that assumes theBaseUtcOffsetDelta
is zero.Also - A much better way to resolve ambiguous and invalid
DateTime
values is to account for both with respect to a given time zone and resolve to aDateTimeOffset
. I've used this approach for some time now with the following extension method on several StackOverflow answers (such as this one) and in real apps. Feel free to use it in revised guidance:Document Details
⚠ Do not edit this section. It is required for learn.microsoft.com ➟ GitHub issue linking.