Re: [code-review] Hello!

Jeff Yoak wrote:

>     Yesterday I uploaded the first version of my first CPAN module, 
> Tie::Hash::Expire, a package providing hash keys that expire and cease 
> to exist after a user-set period.  I would be much obliged if anyone 
> here would take a look.

Your docs don't mention that there's a default timeout.  In particular, 
they don't mention that the default is zero.  I would *expect* the 
default to be for it to behave like a normal hash and to not expire 
anything.  You don't test that default either.

Using keys 'hash' and 'array' in your blessed object is not the best 
choice of names - they're obviously a hash and an array from how you 
create and access them.

Your code might be clearer if instead of storing the creation time of 
entries, you store their desired deletion time, because you check to see 
if something should be deleted more often than you create them.  I 
suppose you might like to refactor that!

Accessing the time/key/value data by numeric indices 0/1/2 is unclear. 
Take a look at the 'constant' pramga for named constant goodness.

>                              I haven't documented this feature as 
> I'm still a little in the air about whether I should retain the 
> implementation that coincidentally makes that possible.

Agreed, it certainly shouldn't be documented.  Letting people think that 
order means anything in a hash is ... unwise.  The implementation is 
quite nice though, once I'd wrapped my head around how it worked.  I'm 
just a bit concerned that it might be a bit too tricksy.

-- 
David Cantrell | Official London Perl Mongers Bad Influence

     Wow, my first sigquoting! I feel so special now!
         -- Dan Sugalski