c# extension methods to convert timezones

Question

I wrote two extension methods to convert times from local time to UTC and back. This is what I have:

public static DateTime ConvertUserTimeToUTCTime(this DateTime TheUserTime, string TheTimezoneID)
{
    TheUserTime = new DateTime(TheUserTime.Year, TheUserTime.Month,
       TheUserTime.Day, TheUserTime.Hour, TheUserTime.Minute,
       TheUserTime.Second, DateTimeKind.Local);

    TimeZoneInfo TheTZ = TimeZoneInfo.FindSystemTimeZoneById(TheTimezoneID);
    TimeSpan TheOffset = TheTZ.GetUtcOffset(TheUserTime);

    DateTimeOffset TheUTCTimeOffset = new DateTimeOffset(
        TheUserTime, TheOffset).ToUniversalTime();

    DateTime TheUTCTime = new DateTime(TheUTCTimeOffset.Year,
        TheUTCTimeOffset.Month, TheUTCTimeOffset.Day, TheUTCTimeOffset.Hour,
        TheUTCTimeOffset.Minute, 0, DateTimeKind.Utc);

   return TheUTCTime;
}




public static DateTime ConvertUTCTimeToUserTime(this DateTime TheUTCTime,
    string TheTimezoneID)
{
    TimeZoneInfo TheTZ = TimeZoneInfo.FindSystemTimeZoneById(TheTimezoneID);

    DateTime UTCTime = new DateTime(TheUTCTime.Year, TheUTCTime.Month, 
       TheUTCTime.Day, TheUTCTime.Hour, TheUTCTime.Minute, 0, DateTimeKind.Utc);

    DateTime TheUserTime = TimeZoneInfo.ConvertTime(UTCTime, TheTZ);

    return TheUserTime;
}

I use these two quite frequently in my app and I was wondering if they're thread safe. Also, would there be any benefit to putting these two method in an abstract class and then having all the classes that involve time operations inherit from this abstract class?

Thanks for your suggestions on this time conversion topic.

I typically put all my extension methods into a single class and include that namespace wherever I need an extension method. This way any DateTime object that may need this extension can access them. I would hate to burn a base class for this since it can be "mixed in" at runtime. — Rob Jefferies, Mar 02 '12 at 18:29

Jon Skeet · Accepted Answer · 2012-03-02T19:16:16.110

5

Yes, they're thread-safe. They're ugly in terms of variable naming (why "The" before everything, and why the Pascal-casing?), and you should consider using DateTime.SpecifyKind, but they're not doing anything with shared state... unless TimeZoneInfo has thread safety problems, it should be fine. (TimeZoneInfo actually specifies that instance members aren't guaranteed to be thread-safe, but it's also noted to be immutable. I'd expect it to be thread-safe.)

You definitely shouldn't put these in an abstract class as a base class - that would be an abuse of inheritance. It doesn't really represent some abstract notion you want to specialize, does it? Extension methods are reasonable here.

You should also think very carefully about how you want the code in the first method to behave in the case of ambiguous or invalid conversions: for example, if the clock goes forward at 1am to 2am, then 1:30am is invalid in that time zone on that day. Likewise if it goes back from 2am to 1am, then 1:30am occurs twice. Check the docs for TimeZoneInfo.GetUtcOffset to make sure that the value returned from it is what you want in those situations.

Finally, consider using Noda Time instead, my alternative .NET date/time API which (I believe) keeps things rather cleaner - and makes your choices around things like date/time conversion very explicit.

(I wrote a blog post about this very topic just the other day, in terms of the choices I've been thinking about for the API. Feedback welcome.)

edited Mar 02 '12 at 19:16

answered Mar 02 '12 at 18:31

Jon Skeet

1,421,763
867
9,128
9,194

Ok, thanks for your thread safety input and for the recommendation of keeping my extension methods as is. This notion of timezones is really hella confusing! Wish there'd be a native .net functionality to switch timezones: supply a datetime and a timezone and get a datetime back. As for the notation, I put The when I'm dealing with an instance of an object; other people write _myVariable and I find that weird. Matter of style I guess. – frenchie Mar 02 '12 at 18:47
@frenchie: It just seems a bit redundant to me - especially when you use "The" several times in a single method invocation. As for "native .NET functionality to switch time zones" - that's exactly what `TimeZoneInfo.ConvertTime` does, but you need to be careful how you use it. This is because `DateTime` is fundamentally broken - see http://noda-time.blogspot.com/2011/08/what-wrong-with-datetime-anyway.html – Jon Skeet Mar 02 '12 at 18:54
@JonSkeet it might be just me, but SourceForge Noda Time link is 404ing. – cadrell0 Mar 02 '12 at 19:15
@cadrell0: Nope, it was a silly typo. Joda Time is joda-time.sf.net; Noda Time is noda-time.googlecode.com. It's not the first time I've done a mix-and-match on them! – Jon Skeet Mar 02 '12 at 19:16
Jon, take a look at this: http://stackoverflow.com/questions/9539561/javascript-timezone-converter – frenchie Mar 02 '12 at 19:59

score 1 · Answer 2 · answered Mar 02 '12 at 18:39

I want to make sure we are talking the same term. Thread Safety is

A piece of code is thread-safe if it only manipulates shared data structures in a manner that guarantees safe execution by multiple threads at the same time. There are various strategies for making thread-safe data structures

Looking at your method the only way it would not be thread safe is if you are changing members of a class or the parameters coming in.

Since these methods are static you cannot be changing members of a class that aspect you are thread safe.

Next thread safety could be violated if you are changing the objects coming into the methods.

Since in your code you are not changing the variables you are fine on that route. Also you could tell by the types you were using. Since strings are immutable they are always thread safe. Since DateTime is a struct that is thread safe.

c# extension methods to convert timezones

2 Answers2