HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Methods creating transform functionality on Collections

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
creatingmethodscollectionstransformfunctionality

Problem

I have written a few simple-to-be-used methods in Java 8, and am wondering what could be improved upon those:

public static > C transform(final Collection collection, final Function mapper) {
    try {
        Objects.requireNonNull(collection);
        Objects.requireNonNull(mapper);
        Class clazz = collection.getClass();
        @SuppressWarnings("unchecked") Collection returnCollection = clazz.newInstance();
        collection.stream().map(mapper).forEach(returnCollection::add);
        @SuppressWarnings("unchecked") C genericReturnCollection = (C)returnCollection;
        return genericReturnCollection;
    } catch (InstantiationException | IllegalAccessException ex) {
        throw new RuntimeException(ex);
    }
}

public static > C transform(final Collection collection, final Function mapper, final Supplier collectionSupplier) {
    Objects.requireNonNull(collection);
    Objects.requireNonNull(mapper);
    Objects.requireNonNull(collectionSupplier);
    return collection.stream().map(mapper).collect(Collectors.toCollection(collectionSupplier));
}


Their purpose is to provide similar functionality as the List.replaceAll() method, but then allowing that the mapped type differs from the original type. Hence it is something that should be in a static factory class, as it will always need to create a new collection.

Some example code:

LinkedList linkedList = new LinkedList<>();
linkedList.add(5);
linkedList.add(10);
linkedList.add(15);
LinkedList resultList = transform(linkedList, i -> (i + 1) + "-" + i);


and

LinkedList linkedList = new LinkedList<>();
linkedList.add(5);
linkedList.add(10);
linkedList.add(15);
LinkedList resultList = transform(linkedList, i -> (i + 1) + "-" + i, LinkedList::new);


Both have as output:


6-5

11-10

16-15

I am aware that the transform with a Supplier as argument is obviously much nicer, however for convienience the single-argument version with only a function is much better.

Solution

I see the Supplier-less option as being a very broken one. Consider the following code:

transform(linkedList.sublist(0, linkedList.size() / 2), i -> (i + 1) + "-" + i);


Or, any of the other collections that do not have a meaningful default constructor:

transform(Arrays.asList(argv), a -> a.toLowerCase());


The code will cause more headaches than the small requirement to add the supplier will require.

The Generics of the mapping are also broken when there is no supplier, because the actual type of the return list should be more specific....

Lose the first static method.

The second method looks fine, except for the ordering of the arguments. I would make the supplier the middle, not the last argument (and I would introduce some newlines to make it not-so-long-of-a-line):

public static > C transform(
          final Collection collection,
          final Supplier collectionSupplier,
          final Function mapper) {
    ....
}


The logical reading of that order is better, take this collection, map to that collection, and use this function to do it..... maybe it is not such a big deal...?

The use-case seems to read better for me:

List lcargs = transform(Arrays.asList(args), ArrayList::new, a -> a.toLowerCase());

Code Snippets

transform(linkedList.sublist(0, linkedList.size() / 2), i -> (i + 1) + "-" + i);
transform(Arrays.asList(argv), a -> a.toLowerCase());
public static <E, R, C extends Collection<R>> C transform(
          final Collection<E> collection,
          final Supplier<C> collectionSupplier,
          final Function<? super E, ? extends R> mapper) {
    ....
}
List<String> lcargs = transform(Arrays.asList(args), ArrayList::new, a -> a.toLowerCase());

Context

StackExchange Code Review Q#47967, answer score: 4

Revisions (0)

No revisions yet.