On Tue, 29 Dec 2020 10:21:07 GMT, Peter Levart <plev...@openjdk.org> wrote:
>> "This message" referred to the entirety of that very comment of mine >> https://github.com/openjdk/jdk/pull/1764#issuecomment-748926986 >> >> I prepended that message with that clause (that is, the wording to the left >> of the colon) to make it clear that I didn't want to review or argue with >> anything said before that in that thread; it seems that clause made more >> harm than good. > > 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. ------------- PR: https://git.openjdk.java.net/jdk/pull/1764
