Cutting it to Pages

I was working on representing a large List of information on a number of different pages, and a colleague of mine suggested that I try to make a generic method for the task I was trying to perform:

private <T> List<List<T>> listSplitter(List<T> originalList, int splitCount) {
    List<List<T>> listOfLists= new ArrayList<>();
    listOfLists.add(new ArrayList<>());

    int originalListSize = originalList.size();     
    int index = 0;
    int pageNumber = 0;
    int numItemsAdded = 0;

    while (index < originalListSize) {
        if (numItemsAdded > splitCount - 1) {
            numItemsAdded = 0;
            pageNumber ++;
            listOfLists.add(new ArrayList<>());
        }
        List activeList = listOfLists.get(pageNumber);
        activeList.add(originalList.get(index));
        numItemsAdded++;
        index++;
    }

    return listOfLists;
}

This correctly splits a list of objects into any number of lists based on the desired number of objects per list. I would love to get feedback on this method, as I am new to Java generics.

Answer

Compiler Warning

This line gives you a compiler warning because it’s not using generics:

List activeList = listOfLists.get(pageNumber);

it should be

List<T> activeList = listOfLists.get(pageNumber);

Empty list input

If the input is an empty list, then the output will be a list containing one empty list. I think it would be more reasonable if the output itself was an empty list.

This can be solved by only adding a list if the number of items added is more than zero.


splitCount <= 0

If I give the method a zero or negative value for splitCount, then the output will look like this:

[[], [0], [1], [2], [3], [4], [5]]

The proper response would be to throw an Exception. That value just shouldn’t be zero or less.


Iterator

As your code is iterating through a list, it could use the Iterator for the list instead of using the indexes. For a LinkedList, the performance of get is very slow, so always using Iterator would give a significantly faster performance for such lists, and is a good practice even on ArrayList.


activeList

Currently you keep track of a ‘pageNumber’ and for each item you add, you get the list with that page number. It would be better to declare the activeList on the function-scope instead of inside the scope of the for-loop. Then you can change the activeList variable when needed, i.e. inside your if statement.


if (numItemsAdded > splitCount - 1) {

This if-statement would be a lot easier to grasp if it was

if (numItemsAdded >= splitCount) {

There is no need to introduce the - 1 when you can just change the operator.


My final code:

With the above in mind, here’s what I ended up with:

private static <T> List<List<T>> listSplitter(List<T> originalList, int resultsPerList) {
    if (resultsPerList <= 0) {
        throw new IllegalArgumentException("resultsPerList must be positive");
    }
    List<List<T>> listOfLists = new ArrayList<>();
    List<T> latestList = new ArrayList<>();
    Iterator<T> iterator = originalList.iterator();
    
    while (iterator.hasNext()) {
        T next = iterator.next();
        if (latestList.size() >= resultsPerList) {
            listOfLists.add(latestList);
            latestList = new ArrayList<T>();
        } 
        latestList.add(next);
    }
    
    if (!latestList.isEmpty()) {
        listOfLists.add(latestList);
    }

    return listOfLists;
}

Attribution
Source : Link , Question Author : bazola , Answer Author : Community

Leave a Comment