Skip to content
This repository has been archived by the owner on Sep 8, 2018. It is now read-only.

added isDST and getAbbreviation to TimeZoneTime #45

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

added isDST and getAbbreviation to TimeZoneTime #45

wants to merge 1 commit into from

Conversation

vogievetsky
Copy link

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
Copy link
Contributor

jgable commented Nov 8, 2013

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
Copy link
Author

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
Copy link

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants