There is a problem on Exercism Java track that requires writing a function to flatten an arbitrarily nested list to a flattened list without null values. For Example
input: [1,[2,3,null,4],[null],5]
output: [1,2,3,4,5]
Here is the solution I wrote for it without using wildcards.
public List flatten(List<Object> two) { Function<Object, List> mapper = item -> (item instanceof List) ? flatten((List) item) : Collections.singletonList(item); return (List) two.stream() .filter(x -> x != null) .map(mapper) .flatMap(x -> x.stream()) .collect(Collectors.toList()); } Here is my solution with the wildcard generics. My IDE was happier with this solution. public List<Object> flatten(List<?> two) { Function<Object, List<?>> mapper = item -> (item instanceof List) ? flatten((List<?>) item) : Collections.singletonList(item); return two.stream() .filter(Objects::nonNull) .map(mapper) .flatMap(List::stream) .collect(Collectors.toList()); }
Is it ok to use the wildcard in this case? Which solution would you prefer? Any other comments about the code are also welcome.
Your answer
Log in to share your answer and help other learners.
The question regarding the java generic list with wildcards vs non generic list -
Your IDE complains about the first one because you're mixing raw types and parameterized types. By changing the raw types to List
Plain old review Method name should tell the caller it does recursive flattening. Did the exercise specify only lists to be flattened instead of all collections? If the former, call it flattenListsRecursively. Parameter name two is confusing. Variable name mapper is a bit generic. Use mapToList or similar instead. There is no limit in the recursion. The last one may be a bit controversial, but my opinion is that if you intend to do a recursive algorithm for generic use you should provide the caller with tools to prevent it from running out of control instead of just relying on an eventual StackOverflowError.
Performance
The stream API requires you to do an awful lot of throwaway singleton list creation. A good old loop with recursion would be much more effective in this case. It may even be a bit more readable as the recursive call is not buried down into a fairly long one line lambda. Remember, streams are like a hammer. It's a handy tool sometimes but hardly the right tool every time.
public static List flattenListsRecursively(List input) {
final List result = new ArrayList<>();
flattenListRecursively(input, result);
return result;
}
private static void flattenListsRecursively(List input, List result) {
for (Object o: input) {
if (o instanceof List) {
flattenListRecursively((List) o, result);
} else if (o != null) {
result.add(o);
}
}
}
If you made the latter public, you could let the caller use the result list as a way to control the execution by, for example, passing a size limited list. But this doesn't solve the stack overflow problem I mentioned earlier as a list of lists that contains itself just goes into a runaway recursion without necessarily affecting the output size.
