Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[COLLECTIONS-800] Adding partitionBalanced(List,int) method #265

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[COLLECTIONS-800] Adding partitionBalanced(List,int) method #265

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 62 additions & 5 deletions src/main/java/org/apache/commons/collections4/ListUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,12 @@ public void visitKeepCommand(final E object) {
private static class Partition<T> extends AbstractList<List<T>> {
private final List<T> list;
private final int size;
private final boolean isBalanced;

private Partition(final List<T> list, final int size) {
private Partition(final List<T> list, final int size, final boolean isBalanced) {
this.list = list;
this.size = size;
this.isBalanced = isBalanced;
}

@Override
Expand All @@ -117,9 +119,25 @@ public List<T> get(final int index) {
throw new IndexOutOfBoundsException("Index " + index + " must be less than size " +
listSize);
}
final int start = index * size;
final int end = Math.min(start + size, list.size());
return list.subList(start, end);
int start;
int currentPartitionSize;
if (isBalanced) {
// evenly distribute partitions
currentPartitionSize = (int) Math.ceil((double) list.size() / (double) listSize);
start = index * currentPartitionSize;
// remainder of above is the threshold for which indices greater than will have one less element
final int threshold = (list.size() % listSize);
// when currentPartitionSize is 1 or threshold is 0 there's nothing to balance
// when index hasn't crossed the threshold we don't need balancing yet
if (currentPartitionSize > 1 && threshold > 0 && index >= threshold) {
start -= (index - threshold); // adjust start as partitions before threshold have one less element
currentPartitionSize--; // currentPartitionSize is decremented as threshold is crossed
Copy link

Choose a reason for hiding this comment

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

Newbie myself, but I wonder if that could be written more simply:

evenlyDistributedSize = ...
if (index < threshold) { // lists before threshold should contain +1
    start = index * (evenlyDistributedSize + 1);
    end = start + (evenlyDistributedSize + 1);
} else {
    start = index * evenlyDistributedSize + threshold; // offset other starting positions by threshold
    end = start + evenlyDistributedSize;
}

...or similar (Note: this is untested). The if condition and subtractions look quite complicated in my opinion.

}
} else {
start = index * size;
currentPartitionSize = Math.min(size, list.size() - start);
}
return list.subList(start, start + currentPartitionSize);
}

@Override
Expand Down Expand Up @@ -486,13 +504,52 @@ public static <E> List<E> longestCommonSubsequence(final List<E> listA, final Li
* @throws NullPointerException if list is null
* @throws IllegalArgumentException if size is not strictly positive
* @since 4.0
* @see ListUtils#partitionBalanced(List, int)
*/
public static <T> List<List<T>> partition(final List<T> list, final int size) {
Objects.requireNonNull(list, "list");
if (size <= 0) {
throw new IllegalArgumentException("Size must be greater than 0");
}
return new Partition<>(list, size);
return new Partition<>(list, size, false);
}

/**
* Returns consecutive {@link List#subList(int, int) sublists} of a
* list, partitioned in a way to balance entries across all sublists. For example,
* partitioning a list containing {@code [a, b, c, d, e]} with a partition
* size of 3 yields {@code [[a, b, c], [d, e]]} -- an outer list containing
* two inner lists of three and two elements, all in the original order. Partitioning
Copy link

Choose a reason for hiding this comment

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

I see why you would use the example from ListUtils.partition, however this could be confusing IMO if you start comparing what method to use and see no difference so far. I would opt for a more clear example here.

* the same input list with a partition size of 4 also yields {@code [[a, b, c], [d, e]]}
* since putting 4 elements in the first partition makes it unbalanced as the last
* partition would only have 1 element.
* <p>
* The outer list is unmodifiable, but reflects the latest state of the
* source list. The inner lists are sublist views of the original list,
* produced on demand using {@link List#subList(int, int)}, and are subject
* to all the usual caveats about modification as explained in that API.
* The size of the produced list is always equals to the size of the list
* produced by using the same input with the {@link ListUtils#partition(List, int)}
* method.
* <p>
* Adapted from http://code.google.com/p/guava-libraries/
*
* @param <T> the element type
* @param list the list to return consecutive balanced sublists of
* @param size the desired maximum size of each sublist
* @return a list of consecutive sublists balanced to have maximum size difference
* of 1 between them
* @throws NullPointerException if list is null
* @throws IllegalArgumentException if size is not strictly positive
* @since 4.5
* @see ListUtils#partition(List, int)
*/
public static <T> List<List<T>> partitionBalanced(final List<T> list, final int size) {
Objects.requireNonNull(list, "list");
if (size <= 0) {
throw new IllegalArgumentException("Size must be greater than 0");
}
return new Partition<>(list, size, true);
}

/**
Expand Down
160 changes: 155 additions & 5 deletions src/test/java/org/apache/commons/collections4/ListUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,13 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import java.util.stream.Stream;

import org.apache.commons.collections4.functors.EqualPredicate;
import org.apache.commons.collections4.list.PredicatedList;
import org.apache.commons.lang3.tuple.Triple;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -323,19 +327,45 @@ public void testLongestCommonSubsequenceWithString() {
}

@Test
@SuppressWarnings("boxing") // OK in test code
public void testPartition() {
final List<Integer> strings = new ArrayList<>();
for (int i = 0; i <= 6; i++) {
strings.add(i);
}
final List<Integer> strings = IntStream.rangeClosed(0, 6).boxed().collect(Collectors.toList());

// [0,1,2,3,4,5,6] -> [[0,1,2],[3,4,5],[6]]
final List<List<Integer>> partition = ListUtils.partition(strings, 3);

assertNotNull(partition);
assertEquals(3, partition.size());
assertEquals(3, partition.get(0).size());
assertEquals(3, partition.get(1).size());
assertEquals(1, partition.get(2).size());

// [0,1,2,3,4,5,6] -> [[0,1,2,3],[4,5,6]]
final List<List<Integer>> partition4 = ListUtils.partition(strings, 4);
assertNotNull(partition4);
assertEquals(2, partition4.size());
assertEquals(4, partition4.get(0).size());
assertEquals(3, partition4.get(1).size());

// [0,1,2,3,4,5,6] -> [[0,1,2,3,4],[5,6]]
final List<List<Integer>> partition5 = ListUtils.partition(strings, 5);
assertNotNull(partition5);
assertEquals(2, partition5.size());
assertEquals(5, partition5.get(0).size());
assertEquals(2, partition5.get(1).size());

// [0,1,2,3,4,5,6] -> [[0,1,2,3,4,5],[6]]
final List<List<Integer>> partition6 = ListUtils.partition(strings, 6);
assertNotNull(partition6);
assertEquals(2, partition6.size());
assertEquals(6, partition6.get(0).size());
assertEquals(1, partition6.get(1).size());

// [0,1,2,3,4,5,6] -> [[0,1,2,3,4,5,6]]
final List<List<Integer>> partition7 = ListUtils.partition(strings, 7);
assertNotNull(partition7);
assertEquals(1, partition7.size());
assertEquals(7, partition7.get(0).size());

try {
ListUtils.partition(null, 3);
fail("failed to check for null argument");
Expand All @@ -351,10 +381,130 @@ public void testPartition() {
fail("failed to check for size argument");
} catch (final IllegalArgumentException e) {}

final List<List<Integer>> partitionMin = ListUtils.partition(strings, 1);
assertEquals(strings.size(), partitionMin.size());
for (int i = 0; i < strings.size(); i++) {
assertEquals(1, partitionMin.get(i).size());
assertEquals(strings.get(i), partitionMin.get(i).get(0));
}

final List<List<Integer>> partitionMax = ListUtils.partition(strings, Integer.MAX_VALUE);
assertEquals(1, partitionMax.size());
assertEquals(strings.size(), partitionMax.get(0).size());
assertEquals(strings, partitionMax.get(0));

// Edge case: partitioning empty list doesn't throw exception
IntStream.rangeClosed(1, 5).forEach(it -> {
final List<List<String>> partitionEmptyList = ListUtils.partition(new ArrayList<>(), it);
assertEquals(0, partitionEmptyList.size());
assertTrue(partitionEmptyList.isEmpty());
});

// Edge case: partitioning single value list doesn't throw exception
IntStream.rangeClosed(1, 5).forEach(it -> {
final List<List<String>> partitionSingleElemList = ListUtils.partition(Collections.singletonList("42"), it);
assertFalse(partitionSingleElemList.isEmpty());
assertEquals(1, partitionSingleElemList.size());
assertEquals("42", partitionSingleElemList.get(0).get(0));
});
}

@Test
public void testPartitionBalanced() {
final List<String> evenList = IntStream.rangeClosed(0, 9).mapToObj(it -> it + "").collect(Collectors.toList());
final List<String> oddList = IntStream.rangeClosed(0, 12).mapToObj(it -> it + "").collect(Collectors.toList());
Stream.of(
Triple.of(evenList, "0|1|2|3|4|5|6|7|8|9", 1),
Triple.of(evenList, "0,1|2,3|4,5|6,7|8,9", 2),
Triple.of(evenList, "0,1,2|3,4,5|6,7|8,9", 3), // balanced
Triple.of(evenList, "0,1,2,3|4,5,6|7,8,9", 4), // balanced
Triple.of(evenList, "0,1,2,3,4|5,6,7,8,9", 5),
Triple.of(evenList, "0,1,2,3,4|5,6,7,8,9", 6), // balanced
Triple.of(evenList, "0,1,2,3,4|5,6,7,8,9", 7), // balanced
Triple.of(evenList, "0,1,2,3,4|5,6,7,8,9", 8), // balanced
Triple.of(evenList, "0,1,2,3,4|5,6,7,8,9", 9), // balanced
Triple.of(evenList, "0,1,2,3,4,5,6,7,8,9", 10),
Triple.of(evenList, "0,1,2,3,4,5,6,7,8,9", 11),
Triple.of(evenList, "0,1,2,3,4,5,6,7,8,9", 12),
Triple.of(oddList, "0|1|2|3|4|5|6|7|8|9|10|11|12", 1),
Triple.of(oddList, "0,1|2,3|4,5|6,7|8,9|10,11|12", 2),
Triple.of(oddList, "0,1,2|3,4,5|6,7,8|9,10|11,12", 3), // balanced
Triple.of(oddList, "0,1,2,3|4,5,6|7,8,9|10,11,12", 4), // balanced
Triple.of(oddList, "0,1,2,3,4|5,6,7,8|9,10,11,12", 5), // balanced
Triple.of(oddList, "0,1,2,3,4|5,6,7,8|9,10,11,12", 6), // balanced
Triple.of(oddList, "0,1,2,3,4,5,6|7,8,9,10,11,12", 7),
Triple.of(oddList, "0,1,2,3,4,5,6|7,8,9,10,11,12", 8), // balanced
Triple.of(oddList, "0,1,2,3,4,5,6|7,8,9,10,11,12", 9), // balanced
Triple.of(oddList, "0,1,2,3,4,5,6|7,8,9,10,11,12", 10), // balanced
Triple.of(oddList, "0,1,2,3,4,5,6|7,8,9,10,11,12", 11), // balanced
Triple.of(oddList, "0,1,2,3,4,5,6|7,8,9,10,11,12", 12), // balanced
Triple.of(oddList, "0,1,2,3,4,5,6,7,8,9,10,11,12", 13),
Triple.of(oddList, "0,1,2,3,4,5,6,7,8,9,10,11,12", 14)
)
.forEach(testCase -> {
final Integer partitionSize = testCase.getRight();
final String expectedResult = testCase.getMiddle();
final List<List<String>> partition = ListUtils.partitionBalanced(testCase.getLeft(), partitionSize);
final List<String> expectedPartitions = Arrays.asList(expectedResult.split("\\|"));
assertEquals(expectedPartitions.size(), partition.size());
for (int i = 0; i < expectedPartitions.size(); i++) {
assertArrayEquals(expectedPartitions.get(i).split(","), partition.get(i).toArray());
}
});

try {
ListUtils.partitionBalanced(null, 3);
fail("failed to check for null argument");
} catch (final NullPointerException ignored) {}

try {
ListUtils.partitionBalanced(oddList, 0);
fail("failed to check for size argument");
} catch (final IllegalArgumentException ignored) {}

try {
ListUtils.partitionBalanced(oddList, -10);
fail("failed to check for size argument");
} catch (final IllegalArgumentException ignored) {}

final List<List<String>> lists = ListUtils.partitionBalanced(oddList, 10);
try {
lists.get(-1);
fail("failed to check for index out of bounds");
} catch (final IndexOutOfBoundsException ignored) {}

try {
lists.get(oddList.size());
fail("failed to check for index out of bounds");
} catch (final IndexOutOfBoundsException ignored) {}

final List<List<String>> partitionMin = ListUtils.partitionBalanced(oddList, 1);
assertEquals(oddList.size(), partitionMin.size());
for (int i = 0; i < oddList.size(); i++) {
assertEquals(1, partitionMin.get(i).size());
assertEquals(oddList.get(i), partitionMin.get(i).get(0));
}

final List<List<String>> partitionMax = ListUtils.partitionBalanced(oddList, Integer.MAX_VALUE);
assertEquals(1, partitionMax.size());
assertEquals(oddList.size(), partitionMax.get(0).size());
assertEquals(oddList, partitionMax.get(0));

// Edge case: partitioning empty list doesn't throw exception
IntStream.rangeClosed(1, 5).forEach(it -> {
final List<List<String>> partitionEmptyList = ListUtils.partitionBalanced(new ArrayList<>(), it);
assertEquals(0, partitionEmptyList.size());
assertTrue(partitionEmptyList.isEmpty());
});

// Edge case: partitioning single value list doesn't throw exception
IntStream.rangeClosed(1, 5).forEach(it -> {
final List<List<String>> partitionSingleElemList =
ListUtils.partitionBalanced(Collections.singletonList("42"), it);
assertFalse(partitionSingleElemList.isEmpty());
assertEquals(1, partitionSingleElemList.size());
assertEquals("42", partitionSingleElemList.get(0).get(0));
});
}

@Test
Expand Down