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.