Skip to content

Add MoreIterables.partition#653

Merged
bulldozer-bot[bot] merged 4 commits into
developfrom
davids/partition
Feb 2, 2026
Merged

Add MoreIterables.partition#653
bulldozer-bot[bot] merged 4 commits into
developfrom
davids/partition

Conversation

@schlosna
Copy link
Copy Markdown
Contributor

@schlosna schlosna commented Jan 23, 2026

Before this PR

After this PR

==COMMIT_MSG==
Similar to Guava Iterables#partition(Iterable, int) and Lists#partition(List, int); however, Guava Iterables#partition(Iterable, int) eagerly allocates array storage leading to excessive allocations with small or empty iterables, while this MoreIterables#partition(Iterable, int) implementation avoids excess allocations and delegates to more efficient Lists#partition(List, int) where possible (e.g. when provided iterable is an ImmutableCollection via asList()) which returns sublist views.

==COMMIT_MSG==

Possible downsides?

@changelog-app
Copy link
Copy Markdown

changelog-app Bot commented Jan 23, 2026

Generate changelog in changelog/@unreleased

Type (Select exactly one)

  • Feature (Adding new functionality)
  • Improvement (Improving existing functionality)
  • Fix (Fixing an issue with existing functionality)
  • Break (Creating a new major version by breaking public APIs)
  • Deprecation (Removing functionality in a non-breaking way)
  • Migration (Automatically moving data/functionality to a new system)

Description

Add MoreIterables.partition

Check the box to generate changelog(s)

  • Generate changelog entry

@schlosna schlosna requested a review from Copilot January 24, 2026 17:38
@changelog-app
Copy link
Copy Markdown

changelog-app Bot commented Jan 24, 2026

Successfully generated changelog entry!

Need to regenerate?

Simply interact with the changelog bot comment again to regenerate these entries.

🔄 Changelog entries were re-generated at Mon, 26 Jan 2026 01:00:57 UTC!


📋Changelog Preview

✨ Features

  • Add MoreIterables.partition (#653)

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@schlosna schlosna requested a review from Copilot January 25, 2026 14:22
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Similar to Guava `Iterables#partition(Iterable, int)` and
`Lists#partition(List, int)`; however, Guava
`Iterables#partition(Iterable, int)` eagerly allocates array storage
leading to excessive allocations with small or empty iterables, while
this `MoreIterables#partition(Iterable, int)` implementation avoids
excess allocations and delegates to more efficient
`Lists#partition(List, int)` where possible (e.g. when provided iterable
is an `ImmutableCollection` via `asList()`) which returns sublist views.
@schlosna schlosna requested a review from Copilot January 26, 2026 00:38
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

aldexis
aldexis previously approved these changes Jan 26, 2026
Copy link
Copy Markdown

@aldexis aldexis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm - just got two questions

private MoreIterables() {}

/**
* Divides an iterable into unmodifiable sublists of the given size (the final iterable may be
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unmodifiable sublists

This is slightly different than the Lists.partition API, correct? (I think this is desirable, but this could in theory yield bugs if we just replace one by the other, albeit rather obvious ones)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct, this version is constrained as providing read-only view of partitions so it is not guaranteed 100% drop-in compatible with Iterables.partition

.allSatisfy(batch -> assertThat(batch)
.asInstanceOf(list(String.class))
.hasSizeLessThanOrEqualTo(partitionSize)
.satisfies(allElements::addAll));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand this .satisfies(allElements::addAll)). What are we verifying here?
Shouldn't we simply check that the (single) partition contains exactly "a" and "b"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adjusted this to be more explicit

Adds 25 new tests covering:
- Invalid input validation (null, zero, negative partition sizes)
- RandomAccess contract verification for various list types
- Additional collection types (TreeSet, ArrayDeque, Vector)
- Large dataset testing (up to 1500+ elements)
- Specific code path coverage (ArrayList creation, fallback paths)

Total test count increased from 17 to 42 tests.

Co-Authored-By: Claude (anthropic.claude-sonnet-4-5-20250929-v1:0) <noreply@anthropic.com>
@policy-bot policy-bot Bot dismissed aldexis’s stale review January 28, 2026 16:56

Invalidated by push of cc8fe25

@schlosna schlosna marked this pull request as ready for review January 28, 2026 17:02
aldexis
aldexis previously approved these changes Jan 29, 2026
Copy link
Copy Markdown

@aldexis aldexis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed merge when ready as I just have one more question

Comment on lines +62 to +63
return Lists.transform(
Lists.partition(Collections.unmodifiableList(list), size), Collections::unmodifiableList);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a need for Collections.unmodifiableList(list) if we're also going to call unmodifiable on each sublist?
And would the main Collections.unmodifiableList(list) actually not make every sublist already unmodifiable?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call, removed

@policy-bot policy-bot Bot dismissed aldexis’s stale review January 30, 2026 19:01

Invalidated by push of abcc47f

@bulldozer-bot bulldozer-bot Bot merged commit 320c1ff into develop Feb 2, 2026
5 checks passed
@bulldozer-bot bulldozer-bot Bot deleted the davids/partition branch February 2, 2026 12:34
@autorelease3
Copy link
Copy Markdown

autorelease3 Bot commented Feb 2, 2026

Released 2.6.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants