Multi statement lambda and for each anti patterns

When I do a code review of lambda/stream code, I am immediately suspicious of two things – block statement lambdas and forEach().

What’s wrong with this? It’s functional programming right?

List<Integer> list = Arrays.asList(1,2,3,4,5,6,7,8,9,10);
		
AtomicInteger sum = new AtomicInteger();
List<Integer> odds = new ArrayList<>();
List<Integer> evens = new ArrayList<>();
		
list.forEach(n -> {
	sum.addAndGet(n);
	if (n % 2 == 0) {
		evens.add(n);
	} else {
		odds.add(n);
	}
});
		
odds.forEach(System.out::println);
System.out.println();
evens.forEach(System.out::println);
System.out.println();
System.out.println(sum);

Well? Not really. It does have a lambda. It doesn’t have a stream, but that’s easy enough to fix: list.stream().forEach(…).

All better? No. Just because you are using a stream doesn’t mean you are doing functional programming. I would much rather see this code as:

List<Integer> list = Arrays.asList(1,2,3,4,5,6,7,8,9,10);
		
		
list.stream()
   .filter(x -> x % 2 == 1)
   .forEach(System.out::println);

System.out.println();

list.stream()
   .filter(x -> x % 2 == 0)
   .forEach(System.out::println);

System.out.println();

list.stream()
   .mapToInt(x -> x)
   .sum();

Yes, I’m still using forEach(). But now I’m using it for one purpose (printing) rather than sticking logic in it.

Whenever I see a forEach() or lambda with more than one statement, my first thought is “could this be clearer or more functional.” Often the answer is yes. Filter(), map() and collect() are you friends.

And if I did need that List?

list.stream()
   .filter(x -> x % 2 == 1)
   .collect(Collectors.toList());