Re: [code-review] Hello!

At 01:50 AM 1/31/2004, David Cantrell wrote:

>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.

That's currently a to-do in the code.  I had wondered what default behavior 
would be least surprising.  I guess instant expiration is clearly a bad 
choice and I could have corrected that before sending it along.  Options 
I've considered are no expiration, as you suggest, and generating an 
error.  An error might not be out of line because this module provides no 
functionality other than expiring hash keys, and tieing a hash to it and 
not specifying an expiration duration is almost certainly an 
error.  Certainly it is a big efficiency cost for no value.


>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!

Interesting idea.  To me that doesn't seem any more clear, but also doesn't 
seem less clear.  I wonder if many people would see it that way.

>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.

Well, that's a good point, and I use the constant pragma for most of my 
code, but wouldn't using constant limit us to 5.6.0 and above or some 
such?  Other than Test::More not being provided standard very far back, I 
think this would work back to any 5.  It doesn't seem the gain would 
warrant cutting off users in the 5.004 neighborhood.


>>                              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.

I feel you on this one, but if it is going to stay this way, I feel it 
should probably be documented.  While it may help entrench some bad ideas 
on the part of confused programmers, it may be of use to some non-confused 
programmers.

Thanks very much for your very helpful comments!

Cheers,
Jeff