On Wed, 12 Oct 2022 13:26:29 GMT, Tagir F. Valeev <tval...@openjdk.org> wrote:
>> Java 17 added RandomGenerator interface. However, existing method >> Collections.shuffle accepts old java.util.Random class. While since Java 19, >> it's possible to use Random.from(RandomGenerator) wrapper, it would be more >> convenient to provide direct overload shuffle(List<?> list, RandomGenerator >> rnd). > > Tagir F. Valeev has updated the pull request incrementally with one > additional commit since the last revision: > > Fixes according to review > > 1. Reduce duplication in tests > 2. Use JumpableGenerator#copy() instead of create(1) in tests, as according > to the spec, seed can be ignored > 3. Simplify documentation for shuffle(List, Random) to avoid duplication. I'm having difficulty imagining how `SequencedCollection::replaceAll` could work or be useful. Obviously it's on List already. It could be added to Deque, because like List, its elements are explicitly positioned (positioning via the API, not by value) and more importantly, the elements are unconstrained. That is, an element could be replaced by any other value and not affect or be affected by any other elements in the collection. However, `LinkedHashSet` is a `Set` and so has a unique-elements constraint. What should `replaceAll` do if the replacement element already exists in the set? (For a shuffle operation, the first swap operation by definition replaces an element with another element that's somewhere else in the set, so this issue arises immediately.) Maybe the unique-elements constraint is suspended for the duration of the call, and it's only enforced at the end of the call. Conceptually that might work, but I have trouble envisioning an implementation that can do that. A `replaceAll` operation on a `NavigableSet` seems right out. The elements are positioned according to their sort order, so there's no notion of updating an element "in-place" at all. The comparison to `Map::replaceAll` doesn't seem to help; that method replaces only the values of the map entries, which are unconstrained. All of the constraints of maps (uniqueness, sort order) are on the keys. For these reasons I don't think it makes sense to couple this PR to JEP 431. ------------- PR: https://git.openjdk.org/jdk/pull/10520
