Asking about the interesting behaviours of TreeMap.putAll

[Charles Lee]

Hi guys,

Sorry for the late reply.

I agree with David. There are two types of "effect" in my mind:
1. "Associates the specified value with the specified key in this map 
(optional operation). If the map previously contained a mapping for the 
key, the old value is replaced by the specified value. (A map m is said 
to contain a mapping for a key k if and only if m.containsKey(k) would 
return true.)". I get it from Map.put.  Maybe Mike and Doug are thinking 
of this. Using the long sentence instead of "effect of put" will have 
the same effect and will be more concise. Only Map needs to be changed, 
It is not hard to do so (maybe...).
2. The actual effect of put which maybe override by users. Since the 
polymorphism, it is very hard to ensure the same "effect of put" without 
calling it.  I have done some grep on the subtype of AbstractMap to find 
that most putAlls are calling overrided put actually if there are no way 
to do a speed up. I think mentioning "put is in the putAll" in somewhere 
is good to avoid the endless loop if someone tries to call putAll in his 
own put. The "implementation notes" maybe a good place.

I totally agree it is not a defect. It only surprises users. Look at the 
test case[1] again, one change makes the difference. If the 
implementation notes of putAll can be mentioned somewhere in the 
TreeMap, it will be more concise.

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



On 03/06/2012 08:13 AM, David Holmes wrote:
> I've been on both sides of these "bugs" over the years. The problem in 
> general is that there is no definition of what "equivalent" means. In 
> this particular case I don't think there was any need for Map to even 
> mention the "equivalence":
>
> "Copies all of the mappings from the specified map to this map"
>
> seems pretty clear cut to me. No need to mention put() at all! Given 
> that put() was mentioned it is not unreasonable to think that there 
> was a reason why it was mentioned - perhaps because the intent was to 
> specify this to actually invoke put()! I know that is not the case but 
> my point is that by mentioning it you raise that possibility.
>
> End result: update CR with this discussion and close as not a defect. 
> Will await further response from Charles before doing that.
>
> Thanks,
> David
>
> On 6/03/2012 3:39 AM, Mike Duigou wrote:
>> I'm in agreement with Doug on this. "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." is not the same 
>> as "The effect of this call is to call put(k, v) on this map once
>> for each mapping from key k to value v in the specified map."
>>
>> For those two statements to have the same interpretation would 
>> require classifying the calling of a method, put(k,v), as an effect. 
>> I'd prefer if only mutations are considered effects.
>>
>> Mike
>>
>> On Mar 5 2012, at 03:10 , Doug Lea wrote:
>>
>>> On 03/05/12 05:53, David Holmes wrote:
>>>> Charles,
>>>>
>>>> I just realized that your webrev is for Map not AbstractMap and 
>>>> that it is Map
>>>> the states putAll does a put() on each entry.
>>>
>>> No, Map says:
>>>
>>> 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.
>>>
>>>
>>> It does NOT say that put is called. There are many similar cases
>>> in Collections, where specs of some methods are stated in terms of
>>> behavioral equivalence to others, with no implication that those
>>> others are actually called. For now-questionable reasons,
>>> Abstract* classes go further than this and state that they actually
>>> perform various calls.
>>>
>>> Concrete classes should and do override inhereited javadoc to kill
>>> these Abstract* descriptions whenever they are overridden
>>> (which sadly often means pasting them in and the deleting
>>> some sentences). TreeMap.putAll does so. So I think that
>>> this is Not A Bug.
>>>
>>> -Doug
>>>
>>>
>>>
>>
>


-- 
Yours Charles