On Fri, 26 Apr 2024 22:27:21 GMT, Chen Liang <li...@openjdk.org> wrote:
>> Please review this patch that: >> 1. Implemented `forEach` to optimize for 1 or 2 element collections. >> 2. Implemented `spliterator` to optimize for a single element. >> >> The default implementations for multiple-element immutable collections are >> fine as-is, specializing implementation doesn't provide much benefit. > > Chen Liang has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains 16 commits: > > - Add test to ensure reproducible iteration order > - Merge branch 'master' of https://github.com/openjdk/jdk into > feature/imm-coll-stream > - Merge branch 'master' of https://github.com/openjdk/jdk into > feature/imm-coll-stream > - Use the improved form in forEach > - Merge branch 'master' of https://github.com/openjdk/jdk into > feature/imm-coll-stream > - Null checks should probably be in the beginning... > - mark implicit null checks > - Merge branch 'master' of https://github.com/openjdk/jdk into > feature/imm-coll-stream > - Copyright year, revert changes for non-few element collections > - Merge branch 'master' of https://github.com/openjdk/jdk into > feature/imm-coll-stream > - ... and 6 more: https://git.openjdk.org/jdk/compare/a920af23...70583024 src/java.base/share/classes/java/util/ImmutableCollections.java line 676: > 674: return Collections.singletonSpliterator(e0); > 675: } > 676: return super.spliterator(); Not a big deal, but I prefer using if/else for situations like this where the then-part and else-part are equal in size and are conceptually at the same level. (As opposed to a check for a quick special case, where an early return is sensible, followed by the main logic at the method's top level.) Code in the rest of the file is similar, I think. (yes, I'm aware that this takes an extra line) src/java.base/share/classes/java/util/ImmutableCollections.java line 934: > 932: return Collections.singletonSpliterator(e0); > 933: } > 934: return super.spliterator(); Prefer if/else, as noted above. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15834#discussion_r1775972767 PR Review Comment: https://git.openjdk.org/jdk/pull/15834#discussion_r1775973150
