Dave already replied while I was driving home so some replies maybe short. :)
> I really like the new is_X subs, although in the case of the
No. :) I agree with Dave.
> i.e. If I have aliased EST to America/New_York, could it return
> "America/New_York" rather than 1? Both are true so it should be
> equivalent, but being able to get the mapping seems useful. Although
> if I had EST => US/Eastern, then what should it return? I argue that
> it should still return America/New_York since that is the base time
> zone.
I was considering a function to do this. If you look... I hid a solution in the
docs. :)
my $my_alias = %{ DateTime::TimeZone::Alias->aliases }->{ EST };
I didn't want a function named alias and another one named aliases. But - how about:
Anyways, I'm open to suggestions for a method name.
> Finally I still think that not throwing an error if add()ing a
> duplicate definition makes sense (although I agree that croak()ing is
> the right thing to do). I would like to be able to do:
>
> --
> my $dta = "DateTime::TimeZone::Alias";
> $dta->add(EST => "US/Eastern");
> ...
> $dta->add(EST => "US/Eastern"); # This should be OK
> $dta->add(EST => "US/Central"); # This should croak
> --
This is specifically why I made the is_N functions public.
if ( ! DateTime::TimeZone::Alias->is_defined( 'EST' ) ) {
DateTime::TimeZone::Alias->add( EST => 'US/Eastern' );
}
IF NOT DEFINED "EST" THEN ADD "EST => 'US/Eastern'".
Self documenting code is highly underrated.
> (In fact an add("America/New_York" should also not die). My theory
> being that people may have a sanity check in a few places to make sure
> the right TZ mappings are defined. For instance at work we use Mason
> running in mod_perl under Apache, so the same environment persists
> across different pages. If one page does the add() I wouldn't want a
> subsequent page to have to jump through hoops to check the definition
> of the alias is correct if doing an equivalent add()... I don't think
> we lose anything by allowing a subsequent add() to silently be skipped
> if it is the same.
This is largely a design issue. You could use my above example. Although I did put a
warning in the docs after you pointed this
out last time. Perhaps I should increase the documentation on this.
> One last thing, I just realized that the subs expect to be called as
> class methods. This is fine unless you want to alias them into your
> package (or accidentally call them like DateTime::TimeZone::Alias::add()).
> The following fails:
> --
> use DateTime::TimeZone::Alias;
> *set_tz = \&DateTime::TimeZone::Alias::set;
> set_tz(EST => "US/Eastern");
> --
See Dave's solution.
> I don't know if it is exactly an error, but it is a little annoying.
> You could "fix" it by checking the argument count but that doesn't
> handle remove()... also it will fail if someone wants to superclass
> this class since add calls set through the class. I have no idea if
> there is a general solution to this problem, someone must have hit it
> before though.
import also calls the set method. The only fix for this would break sub-classing.
> Cool stuff, Nice work!
Thanks.
Sometimes I wonder if English is my first language...
<snip patch>
Thanks again for the good comments.
Cheers,
-J
--
