On Tue, 29 Dec 2020 10:42:18 GMT, Peter Levart <plev...@openjdk.org> wrote:
>> Hi, >> Sorry for joining late to this discussion, but I think that changing this >> method to delegate to c.addAll(Arrays.asList(...)) might not be the best >> thing to do here. Why? >> - This might look as a convenience method, but it takes only 2 characters >> less to type Collections.addAll(c, ...) than it takes to type >> c.addAll(Arrays.asList(...)), so we would basically end up with two ways of >> performing the same thing. I think we can make a method that would be worth >> more than that. >> - Pursuing performance is a noble goal, but performance is not everything. >> Predictability and guarantees are also important. The convenience method, >> being varargs method, can also be invoked with an array in place of varargs >> argument. Current implementation is trusted (part of JDK) and guarantees to >> never modify the passed-in array argument. If this was modified to just >> delegate to Collection.addAll(Arrays.asList(array)), it depends on the >> concrete implementation of the .addAll method of the implementation class of >> the collection. Misbehaving collection implementation could modify the >> passed-in Arrays.asList and such modifications would propagate to the array. >> >> So I would preferably do one of two things here. Either change the javadoc >> to describe current behaviour better, or use some other immutable List >> wrapper different than Arrays.asList() which is mutable. WDYT? > > Hint: you could use > `java.util.ImmutableCollections#listFromTrustedArrayNullsAllowed` if only > this method would allow other types of arrays, not just Object[]... I really > don't know why this restriction couldn't be lifted as the captured array is > fully encapsulated. On a second thought, using `java.util.ImmutableCollections#listFromTrustedArrayNullsAllowed` is not a good idea, since the method expects the passed-in array to be trusted and use of this method in `Collections.addAll(col, array)` would wrap an untrusted array. Surely the resulting List would only be used as an argument to `col.addAll(list)`, but since neither `col` is trusted, it could "steal" the passed-in `list` and use it... Some other implementation of immutable list array wrapper would be needed here. ------------- PR: https://git.openjdk.java.net/jdk/pull/1764
