Use lazy evalution and caching for us_zones in TimeZones

Hi Pratik, thank you for your review and comment.

Ad 1.) OK, I can understand that this does not meet your personal coding standard, or the coding standard of the Rails community. I use REGION_ZONES in reference to the US_ZONES regex; the US_ZONES were already in the code, so I just went along.

I am more than willing to change that and resubmit a changed patch. Just say the word! :-D

Ad 2.) When you call the method us_zones before the patch, it calls all.find_all( |z| z.name =~ US_ZONES) which will go through all time zones and match them against the regex US_ZONES. In an web app which uses time zones a lot this will account for several hundreds (or even thousands) regex operations for a single page view.

After applying my patch, the regex is only evaluated once (upon the first invocation of the us_zones method) and the result stored away in a class attribute named @US_ZONES (which I will have to rename, see above point 1). Subsequent invocations of us_zones will not have to evaluate the regex but use the cached result immediately.

Thus, the computational costs for us_zones is decreased sharply.

Please tell me if I see this wrong - this is not unlikely, and I am always eager to learn.

If we're going to preload us_zones, we need to make it thread-safe -- see TimeZone.all for an example of how to do this, if you're interested in updating this patch.

Thanks for the pointer, Geoff, I will definitely have a look (and a go) at it.

It's probably also worth exploring, is the current method of matching against a regex really all that slow? Maybe a quick benchmark would help justify your effort here.

While regular expressions are by far the fastest way to search for patterns, they nevertheless are slow compared to using pre-computed values.

I really do like the idea of having convenience methods for "regional" or "continental" time zones. I just want to make sure that it is done "right" (no offence meant) and no inefficient code makes it to a release.

Sure, for most applications, the benefits will be small. But consider a web application that makes heavy use of time zones and calls us_zones (or whatever) several times for a certain view - then the benefit of re-using pre-computed results becomes clear.

From the top of my head, I can think of several applications that will benefit: f.e. a calendar application for a globally diversified audience (every time you enter an event, you get to choose which time zone the event will be for).

I will provide an updated patch in a timely manner and will hope for the best :-D

BTW: Making the patch thread-safe in the way you mentioned means pre-computing at the time of class loading - did I get this right?

BTW2: Why did my last comment get marked as spam? Did I use any keywords, or was it just to short?

I'd compute on load them right under "ZONES" and put them in a frozen constant "US_ZONES".

Hi Joshua,

thank you for the advice. It is the same I had in mind. Together with this comment, I upload an updated patch the pre-computes a US_ZONES constant, sorts and freezes it.

Thanks to Geoff for pointing out the shortcomings of my first patch.

BTW: Forget about views calling us_zones several times - this should be clearly cached in the view or the corresponding controller action. But still, consider multiple views calling us_zones on a web app with time zone support.