Asking about the interesting behaviours of TreeMap.putAll

[Charles Lee]

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?

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);
>>>> }
>>>> }
>>>>
>>>
>>
>>
>


-- 
Yours Charles