Asking about the interesting behaviours of TreeMap.putAll

[David Holmes]

On 5/03/2012 7:13 PM, Charles Lee wrote:
> Hi David,
>
> I am sorry for the unclear. I was suggesting to add some implementation
> notes on the TreeMap....
>
> The change as you suggested is great. The patch is @
> http://cr.openjdk.java.net/~littlee/map-putall/webrev.00/
> <http://cr.openjdk.java.net/%7Elittlee/map-putall/webrev.00/> . Is it ok
> for you?

Its okay with me in principle but as this is a spec change it has to go 
through the internal CCC process. And first it needs a bug filed. And 
ideally our collections experts (internal and external) will chime in.

David
-----

> On 03/05/2012 12:11 PM, David Holmes wrote:
>> Hi Charles,
>>
>> I'm not quite sure what you are suggesting. In my opinion all that is
>> needed is for AbstractMap.putAll to read:
>>
>> Copies all of the mappings from the specified map to this map
>> (optional operation). The behavior of this operation is undefined if
>> the specified map is modified while the operation is in progress.
>>
>> This implementation iterates over the specified map's entrySet()
>> collection, and calls this map's put operation once for each entry
>> returned by the iteration.
>> ---
>>
>> If AbstractMap.putAll does not allow for a non-put() based
>> implementation then TreeMap is in violation of the spec. In which case
>> TreeMap should not extend AbstractMap.
>>
>> David
>> -----
>>
>> On 5/03/2012 1:42 PM, Charles Lee wrote:
>>> Hi David,
>>>
>>> I also notice that in the AbstractMap doc, it also says:
>>> "The documentation for each non-abstract method in this class describes
>>> its implementation in detail. Each of these methods may be overridden if
>>> the map being implemented admits a more efficient implementation. "
>>>
>>> If this is the logic[1], shall we add some implementation notes in the
>>> subclass of AbstractMap when some default behaviours have been changed?
>>> From the spec, the only implementation of putAll I can find from the
>>> TreeMap is using the put method.
>>>
>>> [1] The logic means:
>>> a. We have to place the implementation note in every specified method
>>> api
>>> b. The subclass feels free to change the implementation.
>>>
>>> On 03/02/2012 05:02 PM, David Holmes wrote:
>>>> HI Charles,
>>>>
>>>> I tend to agree with you. In this case, in my opinion,
>>>> AbstractMap.putAll has no business saying that it is equivalent to
>>>> calling put() as that should be part of the implementation note, not
>>>> the actual spec. Subclasses should be free to implement putAll in the
>>>> most efficient manner possible as TreeMap does.
>>>>
>>>> David
>>>>
>>>> On 2/03/2012 6:17 PM, Charles Lee wrote:
>>>>> Hi guys,
>>>>>
>>>>> I have a small test case[1] and the two invokes of putAll have
>>>>> different
>>>>> behaviors: the first invocation does not use the override put but the
>>>>> second invocation does.
>>>>> The root cause about this can be find in the TreeMap code:
>>>>>
>>>>> /if (size==0 && mapSize!=0 && map instanceof SortedMap) {
>>>>> Comparator c = ((SortedMap)map).comparator();
>>>>> if (c == comparator || (c != null && c.equals(comparator))) {
>>>>> ++modCount;
>>>>> try {
>>>>> buildFromSorted(mapSize, map.entrySet().iterator(),
>>>>> null, null);
>>>>> } catch (java.io.IOException cannotHappen) {
>>>>> } catch (ClassNotFoundException cannotHappen) {
>>>>> }
>>>>> return;
>>>>> }
>>>>> }/
>>>>>
>>>>> When meet some situations, buildFromSorted will be invoked instead of
>>>>> put. I understand it is a speed up, but it may confuse people: "I need
>>>>> my own put because of something, but interestingly sometimes it
>>>>> will not
>>>>> be called when putAll and I do not find the reason from the api spec."
>>>>>
>>>>> From the api spec of TreeMap's putAll, it says nothing about put. But
>>>>> from the api spec of AbstractMap's putAll and Map's putAll, they said:
>>>>> / "The effect of this call is equivalent to that of calling put(k,
>>>>> v) on
>>>>> this map once for each mapping from key k to value v in the specified
>>>>> map. "
>>>>> /The spec clearly say that, putAll will use put, that means, we can
>>>>> not
>>>>> use a putAll in an override put. Otherwise, it will recursive
>>>>> endlessly.
>>>>> So can I use a putAll in the override put method in an class which
>>>>> extends the TreeMap?
>>>>>
>>>>> [1]
>>>>> public class TreeMapTest<K, V> extends TreeMap<K, V> {
>>>>> @Override
>>>>> public V put(K key, V value) {
>>>>> System.out.println(key + " : " + value);
>>>>> return super.put(key, value);
>>>>> }
>>>>>
>>>>> /**
>>>>> * @param args
>>>>> */
>>>>> public static void main(String[] args) {
>>>>> TreeMapTest<Integer, Integer> mTreeMap = new TreeMapTest<>();
>>>>> TreeMap<Integer, Integer> mt = new TreeMap<>();
>>>>> mt.put(1, 1);
>>>>> mTreeMap.putAll(mt);
>>>>>
>>>>> mTreeMap.clear();
>>>>> mTreeMap.put(2, 2);
>>>>> mTreeMap.putAll(mt);
>>>>> }
>>>>> }
>>>>>
>>>>
>>>
>>>
>>
>
>