Return to Revisions

Two parts to this review:

1. the 1-liner for the newInstance()
2. the general mechanism of the method

newInstance()

This code will work, but only for a subset of Collections. For example, there are many Collection implementations that do not have an accessible default constructor. What if the input collection is:

Collection<String> input = Arrays.asList(new String[]{"hello","world"});

The newInstance call on the class of that List will fail.... because that is a class called java.util.Arrays$ArrayList and it is not public at all...

The 'specification' for this problem is probably contrived, and there is no expectation that it should behave well when the inputs are bad. As a contrived example, I guess contrived code is OK.

Which leads on to the next thing, what is the right thing to do?

Well, I don't think there is a right thing to do. If the input requires the support of bare 'Collection' then you have no hope.

In a real example, I would probably specify the method to allow specific known collections that the program would expect. Then not use the newInstance at all.

Alternatively, I would use the Java-8 style Supplier, and have an input that supplies you with the collection you need, and shift the burden to the user of your method. Something like:

public interface CollectionSupplier<T> {
    Collection<T> supply();
}

and then have your method declared as:

public static <E> ArrayList<Collection<E>> SplitCollection (
        final Collection<E> col, final CollectionSupplier<E> supplier) {
    .....
    Collection<E> colEl = supplier.supply();
    ....
}

That would also remove the ugly exceptions you declare to throw... as it is, I would probably (bad practice) 'try' the whole loop, and 'catch' the ugly exceptions and replace them with a wrapper of an IllegalStateException that explains that an inner Collection could not be instantiated.

General

Overall your method is neat, and well structured. I can't really fault it, given the constraints of the problem.

Update: Actually, the method should have a lower-case 's' to start the name. How did I miss that the first time? Should be splitCollection not SplitCollection

I would prefer that the method declared the input col as final: public static <E> ArrayList<Collection<E>> SplitCollection (final Collection<E> col) because that is a habit I am in...

Typically I would recommend against returning the data typed as an ArrayList because the implementation should not be reflected, it should just be List, but, again, in this case, the specification requires an ArrayList return value.

Bonus

A Java 8 implementation of this would be a good exercise.

This is what I would try:

public static <T, U extends Collection<T>, V extends Collection<T>> ArrayList<V> splitCollection (U col, Supplier<V> supplier) {
    
    return col.stream().map(v -> {
           V subcol = supplier.get();
           subcol.add(v);
           return subcol;
        }).collect(Collectors.toCollection(ArrayList::new));
}

and it would be used like:

public static void main(String[] args) {
    System.out.println("Result:"
       + splitCollection(Arrays.asList("hello","world"), ArrayList::new));
}

which gives the output:

Result:[[hello], [world]]

Note, in the Java 8 example, I allow the sub collection to be any supported type that you can add the content to. So, for the above example, the input is an inaccessible list (no public default constructor), but the supplier gives you what you need.