TimeZone2 - first impressions
Wed, Oct 4 2006 22:26
Kathy Kam asked for feedback
on Timezone2, so tonight I decided to have a bit of a play and these are my first impressions// partial code review:
- ConvertTimeBySystemTimeZoneId could the name be any longer ? I think this should probably just be an overload of ConvertTime .
- When using ConvertTimeBySystemTimeZoneId it would be nice to be able to easily refer to the local time zone, e.g. "local" or similar string inst4ead of having to use ConvertTime with two TimeZones.
- the same for (2) in any of the api that takes a time zone id
- time zone id strings should allow the abbreviated form. The list from time and date .com is a pretty good starting point. (note the forms of CST and EST that refer to Australian times should be ignored as they cause an ambiguity with the American ones, and the correct form of AEST is already listed in that list :) )
- FindSystemTimeZoneById should return the same time zone if called multiple times instead of a different instance each time.
- because of 5, if you try to convert a time using ConvertTimeBySystemtimeZoneId using the local timezone id and a date time with the Kind set as Local, it fails, because of a failed reference check to see if the TimeZone2 is the same as the static Local one that is cached.
- related to 5, if the user wants a new instance of a TimeZone2 a clone method, or a constructor that takes a TiemZone2 etc, should be exposed, although this is only likely to be useful in the CreateCusotmtimeZone overloads as TimeZone2 is practically read only.
- because the static Local is cached, either a method to refresh should be exposed, or a check made in the property get to ensure the Timezone has not changed , or it should listen to system events. However as this is static, listening to system events can have side effects as they will not be released until the app domain is released. My preference would be to make an efficient as possible test in the property get, and leave any caching up to the consumer.
- FindSystemTimeZoneById would be more friendly if it had an enum for the time zones, based on the abbreviations.
- Likewise these api that take a zone Id, should accept the daylight string as well.
- Similar to 9, a list of string constants in a class, such as KnownTimeZones would be very helpful. I hate string literals in my code, and they should only be there because I've stepped outside the box ;)
- It would be nice if .NET distanced itself a bit further from the OS, and provided the basic known time zones as a *backup* should they not be found in the registry. That is the base of this API should be platform independent, but it should use the platform configuration if it is there, just like any "setting" should.
I think that's the majority of the things that came to mind in the very brief play and investigation I had. Other than that the class is very cool. Seems to work well. What I would like to see next are some TimeZone2 operations for DateTime, such as adding n hours to a DateTime and ensuring that is adjusted as needed due to DayLight Savings changes etc.
And of course then the final bit would be to include this with DateTime in a structure that will read and write nicely from XML preserving the time zone (ideally as abbreviated string for known TimeZones), and even have a SQL CLR type ??