Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I created a utility method ListUtils.partition(iterable, size) to partition a potentially large list to a list of smaller lists:

public class ListUtils {
    public static <T> List<List<T>> partition(Iterable<T> iterable, int size) {
        if (iterable == null) {
            return Collections.emptyList();
        }
        if (size < 1) {
            List<T> list = new ArrayList<>();
            for (T item : iterable) {
                list.add(item);
            }
            return Arrays.asList(list);
        }
        List<List<T>> result = new ArrayList<>();
        List<T> segment = new ArrayList<>(size);
        int i = 0;
        for (T item : iterable) {
            segment.add(item);
            if (++i == size) {
                result.add(segment);
                segment = new ArrayList<>(size);
                i = 0;
            }
        }
        if (i > 0) {
            result.add(segment);
        }
        return result;
    }
}

Unit tests:

public class ListUtilsTest {
    private <T> List<List<T>> partition(Iterable<T> iterable, int size) {
        return ListUtils.partition(iterable, size);
    }

    @Test
    public void testPartitionEmpty() {
        assertEquals(Arrays.<List<Object>>asList(), partition(Arrays.asList(), 3));
    }

    @Test
    public void testPartitionNull() {
        assertEquals(Arrays.<List<Object>>asList(), partition(null, 3));
    }

    @Test
    public void testPartitionZeroSize() {
        List<Integer> orig = Arrays.asList(1, 2, 3);
        assertEquals(Arrays.asList(orig), partition(orig, 0));
    }

    @Test
    public void testPartitionNegativeSize() {
        List<Integer> orig = Arrays.asList(1, 2, 3);
        assertEquals(Arrays.asList(orig), partition(orig, -3));
    }

    @Test
    public void testPartitionInts() {
        assertEquals(
                Arrays.asList(
                        Arrays.asList(1, 2),
                        Arrays.asList(3, 4),
                        Arrays.asList(5)),
                partition(Arrays.asList(1, 2, 3, 4, 5), 2));
    }

    @Test
    public void testPartitionStrings() {
        assertEquals(
                Arrays.asList(
                        Arrays.asList("1", "2"),
                        Arrays.asList("3", "4"),
                        Arrays.asList("5")),
                partition(Arrays.asList("1", "2", "3", "4", "5"), 2));
    }

    @Test
    public void testPartition_MultipleOfSize() {
        assertEquals(
                Arrays.asList(
                        Arrays.asList(1, 2),
                        Arrays.asList(3, 4)),
                partition(Arrays.asList(1, 2, 3, 4), 2));
    }

    @Test
    public void testPartition_Size1() {
        assertEquals(
                Arrays.asList(
                        Arrays.asList(1),
                        Arrays.asList(2),
                        Arrays.asList(3),
                        Arrays.asList(4)),
                partition(Arrays.asList(1, 2, 3, 4), 1));
    }

    @Test
    public void testPartition_BiggerSize() {
        List<Integer> orig = Arrays.asList(1, 2, 3, 4);
        assertEquals(
                Arrays.asList(orig),
                partition(orig, orig.size() + 11));
    }
}

Can you improve this? Are there interesting corner cases I might have missed in the tests?

share|improve this question
    
You could overload the method, with one that takes a List instead of an Iterable and return a List of subLists of the original list. –  bowmore Aug 27 at 17:47

3 Answers 3

up vote 4 down vote accepted

I don't agree with your method of handling 0 or negative partition sizes.

A partition is always meant to be a sub-section of a whole. A partition with size 0 is impossible if the whole is non-empty, as there is no way to cut up a whole into sub-sections of size 0. (Note, an empty whole will of course be divisible into partitions of size 0). The same logic applies for negative values. I would think raising a RuntimeException such as an IllegalArgumentException would make more sense in that context. null objects shouldn't be partition-able either, as they are not even empty "wholes".

Now as for handling sub-section sizes larger than the size of your list, you already define that by having incomplete sub-sections if your sub-section size doesn't divide into your container size. You are missing a test for this, however.

A final remark: since Iterable isn't confined to Collection's measly 2 billion item limit for indexing, you might consider using a long for your partition size if you plan to use this for large data sets.

share|improve this answer
    
Sorry, missed that one... crossed it out. –  mleyfman Aug 27 at 7:01
    
I agree about handling size < 1. And I think I'll throw IllegalArgumentException for null iterables too. –  janos Aug 27 at 7:02

You can remove the variable i, which I believe makes it cleaner (though possibly at the cost of some performance).

List<List<T>> result = new ArrayList<>();
List<T> segment = new ArrayList<>(size);
for (T item : iterable) {
    segment.add(item);
    if (segment.size() == size) {
        result.add(segment);
        segment = new ArrayList<>(size);
    }
}
if (segment.size() > 0) {
    result.add(segment);
}
share|improve this answer
    
Nice idea, I think the performance difference should be negligible –  janos Aug 27 at 8:17

Bug:

assertEquals(partition(Collections.emptyList(), 0).size(), partition(Collection.emptyList(),1).size())

fails.

That is, there is a special case in which partitioning a list in partitions equal in size to or greater in size than the size of a list to be partitioned does not equal a result with 1 partition.

share|improve this answer
1  
Indeed there is a bug, well spotted! But he bug is not what you describe. The bug is that when size < 1, partition(emptyList, size) returns an empty-list-within-a-list, but when size >= 1 it returns an empty list. So the behavior is inconsistent. It would make more sense to standardize on the the second behavior: return an empty list, never an empty-list-within-a-list. –  janos Aug 27 at 12:00

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.