Question on HashMap change in 8011200 (original) (raw)

Remi Forax forax at univ-mlv.fr
Thu Jun 27 21:13:09 UTC 2013


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.



More information about the core-libs-dev mailing list