[rfc][icedtea-web] TimedHashMap

[Andrew Azores]

Thanks for the quick review and nice catches!

On 05/09/2014 04:52 PM, Omair Majid wrote:
> * Andrew Azores <aazores at redhat.com> [2014-05-09 16:16]:
>> +++ b/netx/net/sourceforge/jnlp/util/TimedHashMap.java
>> +public class TimedHashMap<K, V> implements Map<K, V> {
>>   
>> +    private class TimedEntry<T> {
> Can you make this class static? Otherwise, every instance of this class
> will hold a reference to the parent TimedHashMap instance.

Good catch, don't know why I missed that.

>
>> +    private static final long DEFAULT_TIMEOUT = 10000000000L; // 10 seconds
> A slightly easier-to-read version of this is to use the TimeUnit class:
>
>      private static final long DEFAULT_TIMEOUT_NANOS = TimeUnit.SECONDS.toNanos(10);

I've never heard of this before and I love it. Much better, thanks!

>
>> +    public TimedHashMap(final long timeout) {
>> +    public void setTimeout(final long timeout) {
> Please make a note of the unit (either in javadocs or in the parameter
> name). Really nice would be using two separate parameters: value (long) and unit
> (TimeUnit).
>
>> +            if ((entry.value == null && value == null) || entry.value.equals(value)) {
> Objects.equals(entry.value, value) does this check in a smaller amount
> of code.

I've even used this before, but forgot about it. Nice!

>
>> +    public V remove(final Object key) {
>> +        return actualMap.remove(key).value;
> Does this throw a NullPointerException if the map is empty or the map
> does not contain the key/value?
>
> Thanks,
> Omair
>

Fixed too.

Thanks,

-- 
Andrew A

-------------- next part --------------
A non-text attachment was scrubbed...
Name: timedhashmap-map-impl-2.patch
Type: text/x-patch
Size: 13803 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140509/fd265198/timedhashmap-map-impl-2-0001.patch>