On Mon, 21 Dec 2020 15:23:06 GMT, Pavel Rappo <pra...@openjdk.org> wrote:
>> @pavelrappo also see this not very old comment: >> https://github.com/spring-projects/spring-framework/pull/24636#pullrequestreview-370684078 > > "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? ------------- PR: https://git.openjdk.java.net/jdk/pull/1764
