Skip to content
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

[API Proposal]: DateTime.ToDateTimeOffset(TimeZoneInfo) #78934

Open
mattjohnsonpint opened this issue Nov 28, 2022 · 9 comments
Open

[API Proposal]: DateTime.ToDateTimeOffset(TimeZoneInfo) #78934

mattjohnsonpint opened this issue Nov 28, 2022 · 9 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.DateTime
Milestone

Comments

@mattjohnsonpint
Copy link
Contributor

mattjohnsonpint commented Nov 28, 2022

Background and motivation

Very commonly, a DateTime needs to be converted to a DateTimeOffset with regard to a specific time zone. There is no built-in mechanism for doing so, and an implementation that handles all edge cases can be error prone.

Indeed, our own docs for how to resolve ambiguous DateTime values has an error in that the example code doesn't account for BaseUtcOffsetDelta. I've reported that in dotnet/docs#32773. We also don't provide a single example for resolving both ambiguous and invalid values.

In general we don't make it easy to convert a DateTime to a DateTimeOffset for any other time zones than local or UTC.

For context, consider a form where the date and time of an event are collected. Depending on design requirements, the associated time zone might also be collected on the form, or it might be derived from the location of the event, or inferred from the user creating it. The best practice in this scenario is not to pre-convert everything to UTC - but rather to store the DateTime and time zone ID, then compute a DateTimeOffset or UTC equivalent when needed.

Also, I've used an extension method for this in at least 5 separate StackOverflow answers for different scenarios.

Each of those has additional scenario-specific code, but they all use the same ToDateTimeOffset extension method that accounts for invalid and ambiguous times, with regard to a specific time zone, and respects DateTimeKind properly as well. I believe it should be baked in, and perhaps my implementation is suitable (or a good start).

Also note that in all these scenarios, the preference is to use the daylight time for both ambiguous and invalid resolution. The reason for this is that the daylight instance comes first sequentially. This could be optional, but should be the default - as it is well established in scheduling systems where such operations are prevalent.

API Proposal

namespace System;

public struct DateTime
{
    public DateTimeOffset ToDateTimeOffset(TimeZoneInfo timeZone);
}

API Usage

using System.Globalization;

// New York, USA (Eastern Time). EST = UTC-05:00, EDT = UTC-04:00
TimeZoneInfo tz = TimeZoneInfo.FindSystemTimeZoneById("America/New_York");

// Simple case  (00:00 ET becomes 00:00 EST)
DateTime dt1 = DateTime.ParseExact("2022-12-31T00:00:00", "s", CultureInfo.InvariantCulture);
DateTimeOffset dto1 = dt1.ToDateTimeOffset(tz);
Console.WriteLine(dto1.ToString("o", CultureInfo.InvariantCulture)); // 2022-12-31T00:00:00.0000000-05:00

// Example that resolves ambiguous time (1:30 ET becomes 1:30 EDT)
DateTime dt2 = DateTime.ParseExact("2022-11-06T01:30:00", "s", CultureInfo.InvariantCulture);
DateTimeOffset dto2 = dt2.ToDateTimeOffset(tz);
Console.WriteLine(dto2.ToString("o", CultureInfo.InvariantCulture)); // 2022-11-06T01:30:00.0000000-04:00

// Example that resolves invalid time (2:30 ET becomes 3:30 EDT)
DateTime dt3 = DateTime.ParseExact("2022-03-13T02:30:00", "s", CultureInfo.InvariantCulture);
DateTimeOffset dto3 = dt3.ToDateTimeOffset(tz);
Console.WriteLine(dto3.ToString("o", CultureInfo.InvariantCulture)); // 2022-03-13T03:30:00.0000000-04:00

Alternative Designs

This is more verbose, but would do the same and keep it off the DateTime object.

namespace System;

public class TimeZoneInfo
{
    public static DateTimeOffset ConvertDateTimeToDateTimeOffset(DateTime dateTime, TimeZoneInfo timeZone);
}

Or it could be a static method on DateTimeOffset:

namespace System;

public struct DateTimeOffset
{
    public static DateTimeOffset FromDateTime(DateTime dateTime, TimeZoneInfo timeZone);
}

Risks

No response

@mattjohnsonpint mattjohnsonpint added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Nov 28, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 28, 2022
@mattjohnsonpint
Copy link
Contributor Author

@tarekgh

@mattjohnsonpint
Copy link
Contributor Author

For consideration, here is the extension method I've been using and suggesting to others. The actual implementation could deviate from this, but would need to handle the same cases outlined.

public static DateTimeOffset ToDateTimeOffset(this DateTime dt, TimeZoneInfo tz)
{
    if (dt.Kind != DateTimeKind.Unspecified)
    {
        // Handle UTC or Local kinds (regular and hidden 4th kind)
        DateTimeOffset dto = new DateTimeOffset(dt.ToUniversalTime(), TimeSpan.Zero);
        return TimeZoneInfo.ConvertTime(dto, tz);
    }

    if (tz.IsAmbiguousTime(dt))
    {
        // Prefer the daylight offset, because it comes first sequentially (1:30 ET becomes 1:30 EDT)
        TimeSpan[] offsets = tz.GetAmbiguousTimeOffsets(dt);
        TimeSpan offset = offsets[0] > offsets[1] ? offsets[0] : offsets[1];
        return new DateTimeOffset(dt, offset);
    }

    if (tz.IsInvalidTime(dt))
    {
        // Advance by the gap, and return with the daylight offset  (2:30 ET becomes 3:30 EDT)
        TimeSpan[] offsets = { tz.GetUtcOffset(dt.AddDays(-1)), tz.GetUtcOffset(dt.AddDays(1)) };
        TimeSpan gap = offsets[1] - offsets[0];
        return new DateTimeOffset(dt.Add(gap), offsets[1]);
    }

    // Simple case
    return new DateTimeOffset(dt, tz.GetUtcOffset(dt));
}

@tarekgh tarekgh added this to the 8.0.0 milestone Nov 28, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Nov 28, 2022
@tarekgh tarekgh modified the milestones: 8.0.0, Future Nov 28, 2022
@mattjohnsonpint
Copy link
Contributor Author

Alternative name: WithOffsetForTimeZone

@mattjohnsonpint
Copy link
Contributor Author

Another example use case: https://stackoverflow.com/a/77294370/634824

@mattjohnsonpint
Copy link
Contributor Author

@tarekgh @danmoseley - Can we get some eyes on this for .NET 9? It's very much needed, and there is demand for it, as evidenced by the StackOverflow links above. Thanks.

@tarekgh tarekgh modified the milestones: Future, 9.0.0 Jan 29, 2024
@mattjohnsonpint
Copy link
Contributor Author

I also just added another alternative API to the top, as it could be a factory method on DateTimeOffset.

I was also thinking, with any of these, it might be worth having two methods - one that takes a TimeZoneInfo for the scenarios proposed, and one that takes a TimeSpan for fixed offsets. In other words:

namespace System;

public struct DateTimeOffset
{
    public static DateTimeOffset FromDateTime(DateTime dateTime, TimeZoneInfo timeZone);
    public static DateTimeOffset FromDateTime(DateTime dateTime, TimeSpan offset);
}

@Clockwork-Muse
Copy link
Contributor

.... we really need to get our own DateTimeZoned type....

@tarekgh
Copy link
Member

tarekgh commented Jan 30, 2024

public static DateTimeOffset FromDateTime(DateTime dateTime, TimeSpan offset);

This will look very weird because DateTimeOffset already have a constructor which takes DateTime and TimeSpan offset.

I am leaning to the original proposal but I want to give more control for the user on the behavior. What do you think about:

// Method defined in DateTime type.
public static DateTimeOffset ToDateTimeOffset(TimeZoneInfo tz = TimeZoneInfo.Local, bool useDaylightOffsetWhenAmbiguous = true, bool throwWhenInvalid = false)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.DateTime
Projects
None yet
Development

No branches or pull requests

3 participants