sproutsocial / walltime-js

A JavaScript library for easily translating a UTC time to a "Wall Time" for a particular time zone.
MIT License
121 stars 12 forks source link

added isDST and getAbbreviation to TimeZoneTime #45

Open vogievetsky opened 10 years ago

vogievetsky commented 10 years ago

Hi, Just found this lib and I love it.

I am a timezone-js refugee, I hope I can find asylum here.

Wanted to add a couple of methods that I need. Let me know if this contribution should be done in a different way; I will gladly update it. My editor ate some trailing whitespace (LMK if that is a problem), I would recommend adding a lint rule to forbid them.

Also, what is the point of WallTimeDate? It seems like an unused (and wrong, see line 103) duplicate of TimeZoneTime, should it be removed?

Best regards, Vadim

jgable commented 10 years ago

This looks good. Thanks for putting this together.

The only thing I'm worried about is that rules specify what the format of that abbreviation should be explicitly. Check this area of the northamerica timezone file for instance.

I'm happy to merge this as a stop gap, but taking into account those abbreviations from the rules values would be ideal if you have the time.

vogievetsky commented 10 years ago

I appreciate the feedback - please always err on the side of strict in code reviews. I am still learning about how the olson files work and this is great experience. Please do not merge, I will update the PR.

mattjohnsonpint commented 10 years ago

One thing to remember is that the abbreviations are not uniform. So while some places call it "daylight time", others call it "summer time". For example, in the UK, the standard time is abbreviated "GMT" (Greenwich Mean Time) and the daylight time is abbreviated "BST" (British Summer Time).

A better mechanism would be to ignore the abbreviation and just check for which of two rules has a larger offset. Test Jan 1 and July 1. If their offsets are the same, then there is no DST for this zone (return false). Otherwise, if the offset of your test date matches the smaller of the two offsets, then DST is in effect (return true), or if it matches the larger of the two offsets then standard time is in effect (return false).

A more reliable way is with the following algorithm (pseduocode):

let A = offset at the test date let B = offset in the rule prior to the test date, or A if none. let C = offset in the rule after the test date, or A if none. let D = smallest of (B,C) if A == D return false, else return true

Another way would be to get B and C at specific dates that are safely in the middle of the range for most time zones, such as Jan 1 and July 1. I'd use that approach if I didn't have access to the real data.