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]: TryFindSystemTimeZoneById #66928

Closed
danielchalmers opened this issue Mar 21, 2022 · 8 comments · Fixed by #88071
Closed

[API Proposal]: TryFindSystemTimeZoneById #66928

danielchalmers opened this issue Mar 21, 2022 · 8 comments · Fixed by #88071
Labels
api-approved API was approved in API review, it can be implemented area-System.DateTime
Milestone

Comments

@danielchalmers
Copy link
Contributor

danielchalmers commented Mar 21, 2022

Background and motivation

The method for finding a timezone by a string, TimeZoneInfo.FindSystemTimeZoneById, throws a TimeZoneNotFoundException if it can't find one. AFAIK there isn't a non-throwing method in the runtime.

I use a version of the method below to attempt to find a time zone or fallback to local time:

public static TimeZoneInfo GetTimeZone() =>
DateTimeUtil.TryGetTimeZoneById(Settings.Default.TimeZone, out var timeZoneInfo) ? timeZoneInfo : TimeZoneInfo.Local;

This is of course still possible using the current method but it's not as clean IMO.

API Proposal

namespace System
{
    public class TimeZoneInfo
    {
        public static bool TryFindSystemTimeZoneById(string id, out TimeZoneInfo timeZoneInfo)
        {
            try
            {
                timeZoneInfo = TimeZoneInfo.FindSystemTimeZoneById(id);
                return true;
            }
            catch (TimeZoneNotFoundException)
            {
                timeZoneInfo = null;
                return false;
            }
        }
    }
}

API Usage

// Ask user for timezone.
Console.WriteLine("Enter a timezone");
var tz = Console.ReadLine();

// Validate.
if(TimeZoneInfo.TryFindSystemTimeZoneById(tz, out var tzInfo))
    Console.WriteLine("Found timezone");
else
    Console.WriteLine("Invalid timezone");
// Ask user for timezone.
Console.WriteLine("Enter a timezone");
var tz = Console.ReadLine();

// Try to find timezone or fall back to local time.
var finalTimeZone = TimeZoneInfo.TryFindTimeZoneById(tz, out var tzInfo) ? tzInfo : TimeZoneInfo.Local;
Console.WriteLine("Using " + finalTimeZone.DisplayName);

Alternative Designs

No response

Risks

No response

@danielchalmers danielchalmers added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Mar 21, 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.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Mar 21, 2022
@ghost
Copy link

ghost commented Mar 21, 2022

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

The method for finding a timezone by a string (TimeZoneInfo.FindSystemTimeZoneById) throws a TimeZoneNotFoundException if it can't find one and AFAIK there isn't a non-throwing method for it in the runtime.

I use a version of the method below to attempt to find a timezone from a string, and fallback to local time if one's not found:

public static TimeZoneInfo GetTimeZone() =>
DateTimeUtil.TryGetTimeZoneById(Settings.Default.TimeZone, out var timeZoneInfo) ? timeZoneInfo : TimeZoneInfo.Local;

This is of course still possible using the current method but it's less clean (IMO).

API Proposal

namespace System
{
    public class TimeZoneInfo
    {
        public static bool TryFindSystemTimeZoneById(string id, out TimeZoneInfo timeZoneInfo)
        {
            try
            {
                timeZoneInfo = TimeZoneInfo.FindSystemTimeZoneById(id);
                return true;
            }
            catch (TimeZoneNotFoundException)
            {
                timeZoneInfo = null;
                return false;
            }
        }
    }
}

API Usage

// Ask user for timezone
Console.WriteLine("Enter a timezone");
var tz = Console.ReadLine();

// Validate
if(TimeZoneInfo.TryFindSystemTimeZoneById(tz, out var tzInfo))
    Console.WriteLine("Using entered timezone");
else
    Console.WriteLine("Invalid timezone");
// Ask user for timezone
Console.WriteLine("Enter a timezone");
var tz = Console.ReadLine();

// Try to find timezone or fall back to local time
var finalTimeZone = TimeZoneInfo.TryFindTimeZoneById(tz, out var tzInfo) ? tzInfo : TimeZoneInfo.Local;
Console.WriteLine("Using " + finalTimeZone.DisplayName);

Alternative Designs

No response

Risks

Same risks as adding any new API

Author: danielchalmers
Assignees: -
Labels:

api-suggestion, area-System.Runtime, untriaged

Milestone: -

@pinkfloydx33
Copy link

I implemented the very same workaround this past week. I was surprised there isnt a Try-/non-throwing variant. We accept the TZ name as user-provided input so can't rely on it being valid. The workaround isn't that bad, but it'd be nice to have a way to do a lookup without the exception.

@tannergooding tannergooding added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed untriaged New issue has not been triaged by the area owner labels Jul 15, 2022
@tannergooding tannergooding added this to the Future milestone Jul 15, 2022
@tannergooding
Copy link
Member

CC. @tarekgh, could you provide insight into whether this is something we should look at providing?

@tarekgh
Copy link
Member

tarekgh commented Aug 26, 2022

@tannergooding this is a nice to have helper method but not necessary blocking any scenario. It is easy to have the apps adding extension method that does that.

@tannergooding
Copy link
Member

Do you think this is in the right shape for us to mark api-ready-for-review then or are there any other concerns with the current proposal?

@tarekgh
Copy link
Member

tarekgh commented Aug 26, 2022

The API shape looks good to me.

@dakersnar dakersnar added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration labels Aug 30, 2022
@bartonjs
Copy link
Member

bartonjs commented May 18, 2023

Video

Looks good as proposed (ignoring that the body says to do a try/catch).

namespace System
{
    public class TimeZoneInfo
    {
        public static bool TryFindSystemTimeZoneById(string id, out TimeZoneInfo timeZoneInfo);
    }
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels May 18, 2023
AlexRadch added a commit to AlexRadch/runtime that referenced this issue Jun 26, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 26, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 29, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jul 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.DateTime
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants