search

sampottinger / co_opencampaigndata

API service for serving Colorado TRACER data for opencampaigndata.org
GNU General Public License v3.0
10 stars 1 forks source link

Don't think we need Moment, just try to make a Date. #46

Closed trinary closed 11 years ago

trinary commented 11 years ago

Date copy-constructs given an existing Date, I think type-checking vs throwing away a temporary is a toss-up performance wise. Until you start providing custom formats, Moment acts like Date given an argument to constructor, and we don't need the overhead.

Was going to go with epoch 0 as the default value, but went with undefined, input on that call appreciated. Epoch 0 lets people parse naturally, but gives a "real" value that isn't true and will throw off analysis without further intervention. Undef will force people to try/catch around Date construction which is inconvenient, especially if you're relying on libraries. Can easily change it to a static epoch 0 string.

Given the data I've seen, we'll have mixed results with this. Integers in the 800,000 range I've seen a few times, which will give us milliseconds after epoch, obviously incorrect (and clearly aren't second-resolution epoch times either). Strings are dealt with via Date.parse (http://tools.ietf.org/html/rfc2822#page-14). If we want to get more sophisticated than that we'll have to start trying format string heuristics via Moment.

Thoughts?

sampottinger commented 11 years ago

Ha ha! Straightforward. Looks good to me. When you are ready, go ahead and merge / deploy. Thanks for taking care of this.

trinary commented 11 years ago

Done, that fixed the 3 production queries I had that were breaking.