TreeMap.putAll has undocumented behavior which has different side effect based on parameter

[Mike Duigou]

I am in agreement with David that the TreeMap documentation seems to adequately cover this case. The only possible addition would be to explicitly state that put() might not be called by putAll() or add documentation to Map indicating that if put() is overridden then putAll() must also be overridden.

Mike

On Oct 14 2012, at 23:45 , David Holmes wrote:

> Hi Sean,
> 
> On 15/10/2012 1:50 PM, Sean Chou wrote:
>> Hi all,
>> 
>>     It is found TreeMap.putAll method may or may not invoke
>> TreeMap.put method based on parameter. If TreeMap is extended and the
>> put method is override, it may behave different from what is expected.
>>   This behavior should be documented.
> 
> TreeMap.putAll doesn't state that it invokes put:
> 
> "Copies all of the mappings from the specified map to this map. These mappings replace any mappings that this map had for any of the keys currently in the specified map."
> 
> ---
> 
> As you can tell from the implementation, if dealing with a compatible SortedMap there are more efficient ways to add the entries than iterating through and doing a put().
> 
> Further, as has been discussed in the past, when a method, like Map.putAll is defined as "The effect of this call is equivalent to that of calling put(k, v) for each mapping ... in the specified map" it does not mean that the implementation has to actually call put() for each item.
> 
> David
> -----
> 
>> 
>>     The following test case tries to putAll a hashamp and a treemap in
>> a class extended from TreeMap.
>> 
>> /////////////// Testcase ////////////////////
>> /*
>>  * Copyright (c) 2012 Oracle and/or its affiliates. All rights reserved.
>>  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>  *
>>  * This code is free software; you can redistribute it and/or modify it
>>  * under the terms of the GNU General Public License version 2 only, as
>>  * published by the Free Software Foundation.
>>  *
>>  * This code is distributed in the hope that it will be useful, but WITHOUT
>>  * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>>  * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
>>  * version 2 for more details (a copy is included in the LICENSE file that
>>  * accompanied this code).
>>  *
>>  * You should have received a copy of the GNU General Public License version
>>  * 2 along with this work; if not, write to the Free Software Foundation,
>>  * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
>>  *
>>  * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
>>  * or visit www.oracle.com if you need additional information or have any
>>  * questions.
>>  */
>> 
>> /*
>>  * Portions Copyright (c) 2012 IBM Corporation
>>  */
>> 
>> import java.util.HashMap;
>> import java.util.TreeMap;
>> 
>> public class TreeMapTest {
>> 
>>     static class MyTreeMap extends TreeMap<String, String>  {
>>         public String put(String key, String value) {
>>             System.out.println("Override put is invoked in MyTreeMap");
>>             return super.put(key, value);
>>         }
>>     }
>> 
>> 
>>     public static void main(String[] args) {
>>         TreeMap<String, String>  tm = new TreeMap<>();
>>         tm.put("treeMap", "treeMap");
>> 
>>         HashMap<String, String>  hm = new HashMap<>();
>>         hm.put("hashMap", "hashMap");
>> 
>>         System.out.println("Test started, the override put method in
>> MyTreeMap " +
>>         		            "is not invoked when putAll a TreeMap, but " +
>>         		            "it is invoked when putAll a HashMap");
>> 
>>         TreeMap<String, String>  treeMap = new MyTreeMap();
>>         TreeMap<String, String>  treeMap2 = new MyTreeMap();
>> 
>>         System.out.println("putAll treemap:");
>>         treeMap.putAll(tm);
>>         System.out.println("putAll hashmap:");
>>         treeMap2.putAll(hm);
>>     }
>> 
>> }
>> 
>> 
>> 
>>