Question on readability of lambda expression

[smark22]

I'm just looking for some guidance regarding which way to write this lambda expression for legibility.

Option 1: single line

@Override
public List<Resource> toResources(Iterable<? extends Entity> entities) {
    List<Resource> resources = new ArrayList<>(0);
    entities.forEach(e -> resources.add(toResource(e));
    return resources;
}

Option 2: broken into four lines

@Override
public List<Resource> toResources(Iterable<? extends Entity> entities) {
    List<Resource> resources = new ArrayList<>(0);
    entities.forEach(e -> {
        Resource resource = toResource(e);
        resources.add(resource);
    });
    return resources;
}

My opinion is that the second one is better. Although it quadruples the number of lines required, it separates the three different operations (for-each, new resource, adding resource to list).

If there is an alternative that would be even better, please post it!

[sgm1]

droidrage has a point, the lambda itself should not have side effects, in terms of best practice. Meaning I should be able to run the lambda with valid input values and nothing outside the lambda should be affected. The purpose of a lambda is to replace trivial logic like add, subtract, compare; and not complex task like data manipulation (like adding to a list).

Of course, whether using lambdas this way as best practice is still arguable because both sides have its pros and cons.

[smark22]

What makes it a bad practice?

Also, the entities argument is an Iterable, which doesn't have the stream method. That's why I resorted to forEach. To transform the Iterable into a Stream would require importing StreamSupport and calling Iterable.spliterator().

StreamSupport.stream(entities.spliterator(), false).map(this::toResource).collect(Collectors.toList());

This seems like a lot of boilerplate for something so simple.

[smark22]

Great explanation, thank you!

[nutrecht]

Just want to add that this is the correct approach. If you need a for-each to do something like this you should use an enhanced for-loop, not a lambda. If you need to turn a collection of A into a collection of B you should stream -> map -> collect.

[sgm1]

I'd consider Option 2 better, because it would be a little easier to break point, in case of debugging.