Question on HashMap change in 8011200

[Remi Forax]

On 06/27/2013 10:02 AM, Shi Jun Zhang wrote:
> Hi,
>
> There are some isEmpty() check added into get/remove methods since 
> 8011200 to return directly if HashMap is empty. However isEmpty is a 
> non-final public method which can be overridden by subclass. If the 
> subclass defines isEmpty differently from HashMap, it would cause 
> problem while getting or removing elements.

yes, it's a bug.
Could you report it ?

Rémi

>
> For example, here is a simple test case, the subclass of HashMap 
> always has at least one key value pair so isEmpty will never be false.
>
> /////////////////////////////////////////////////////////////////////////////// 
>
>
> import java.util.HashMap;
>
>
> public class HashMapTest {
>     public static class NotEmptyHashMap<K,V> extends HashMap<K,V> {
>         private K alwaysExistingKey;
>         private V alwaysExistingValue;
>
>         @Override
>         public V get(Object key) {
>             if (key == alwaysExistingKey) {
>                 return alwaysExistingValue;
>             }
>             return super.get(key);
>         }
>
>         @Override
>         public int size() {
>             return super.size() + 1;
>         }
>
>         @Override
>         public boolean isEmpty() {
>             return size() == 0;
>         }
>     }
>
>     public static void main(String[] args) {
>         NotEmptyHashMap<String, String> map = new NotEmptyHashMap<>();
>         map.get("key");
>     }
> }
> ////////////////////////////////////////////////////////////////////////// 
>
>
> The test can end successfully before 8011200 but it will throw 
> ArrayIndexOutOfBoundsException after 8011200. The reason is isEmpty() 
> check in HashMap.getEntry() method gets passed but the actual table 
> length is 0. I think the real intention of isEmpty() check is to check 
> whether size in HashMap is 0 but not whether the subclass is empty, so 
> I suggest to use "size == 0" instead of "isEmpty()" in get/remove 
> methods.
>
> Do you think this is a bug or a wrong usage of extending HashMap? I 
> find there is a similar usage in javax.swing.MultiUIDefaults which 
> extends Hashtable.
>
>