I think DateTime::TimeZone::Alias 0.03 is really looking good. The
docs are excellent (small patch to fix typos included below).
I really like the new is_X subs, although in the case of the
is_alias() would it make sense to return the target of the alias?
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.
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
--
(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.
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");
--
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.
Cool stuff, Nice work!
-ben
The POD diff:
--
--- Alias.pod.orig 2003-06-16 21:52:23.000000000 -0400
+++ Alias.pod 2003-06-16 21:52:59.000000000 -0400
@@ -48,7 +48,7 @@
Accepts a hash representing an alias and the timezone it should map too.
If an invalid timezone value is passed an exception is thrown.
It is possible to create an alias to another alias. This method will
-redefined an existing alias or add a new alias that overrides an existing
+redefine an existing alias or add a new alias that overrides an existing
timezone.
=item * add( alias => timezone, ... )
@@ -56,7 +56,7 @@
Accepts a hash representing an alias and the timezone it should map too.
If an invalid timezone value is passed an exception is thrown.
It is possible to create an alias to another alias.
-This method will _not_ redefined an existing alias or allow you remap a
+This method will _not_ redefine an existing alias or allow you remap a
timezone. For redefinitions see the C<set> method.
=item * remove( alias, ... )
