Hi Roman, addAll() returns a boolean that will be not easy to compute given that forEach() takes a lambda (that can only capture local values and not change them).
and the javadoc of AbstractCollection clearly states that addAll has to use an iterator if implemented https://docs.oracle.com/en/java/javase/13/docs/api/java.base/java/util/AbstractCollection.html#addAll(java.util.Collection) regards, Rémi ----- Mail original ----- > De: "Roman Leventov" <[email protected]> > À: "Pavel Rappo" <[email protected]> > Cc: "core-libs-dev" <[email protected]> > Envoyé: Mercredi 1 Janvier 2020 16:43:22 > Objet: Re: Note bulk methods in Javadocs for Collections.synchronizedXxx() > Maybe it also makes sense to modify common JDK's impls of addAll(), remove > all(), and putAll() to make use of forEach() on the argument collection? > > On Sat, 28 Dec 2019, 18:23 Pavel Rappo, <[email protected]> wrote: > >> If we were to add something like that to the documentation, it would >> probably go >> under @apiNote (section "pitfalls", hehe). >> >> As you said, the spec says just enough to infer that. So, strictly >> speaking, >> there's no need to add that note. After all, there are infinite ways to use >> these, synchronized, collections incorrectly and only one way to use them >> right. >> >> Maybe we could tackle this (and similar requests in the future) by adding, >> once >> and for all, something like: >> >> Consider using concurrent collections instead, such as those that can >> be >> found in the java.util.concurrent package. >> >> -Pavel >> >> > On 28 Dec 2019, at 09:50, Roman Leventov <[email protected]> wrote: >> > >> > I think Javadocs for Collections.synchronizedXxx() should mention not >> > only "traversing it via Iterator, Spliterator or Stream" as something >> that >> > must be synchronized externally, but also cases when a synchronized >> > collection is an argument to a bulk collection method, including: >> > - new ArrayList(syncList), new HashSet(syncSet), etc. >> > - List.copyOf(syncList) >> > - otherColl.addAll(syncList), otherColl.removeAll(syncList), >> > otherMap.putAll(syncMap). >> > >> > Though this is logically inferable, this is far from being obvious and >> > apparent. >>
