Java streams map with sideeffect and collect or foreach and populate a result-list

Question

I have a piece of code that looks something like this.

I have read two contradicting(?) "rules" regarding this.

• That .map should not have side effects
• That .foreach should not update a mutable variable (so if I refactor to use foreach and populate a result list, then that breaks that) as mentioned in http://files.zeroturnaround.com/pdf/zt_java8_streams_cheat_sheet.pdf

How can I solve it so I use streams and still returns a list, or should I simply skip streams?

@Transactional
public Collection<Thing> save(Collection<Thing> things) {
    return things.stream().map(this::save).collect(Collectors.toList());
}

@Transactional
public Thing save(Thing thing) {
    // org.springframework.data.repository.CrudRepository.save 
    // Saves a given entity. Use the returned instance for further operations as the save operation might have changed the entity instance completely.
    Thing saved = thingRepo.save(thing);
    return saved;
}

Answer 1

Doesn't that paper say shared mutable state? In your case if you declare the list inside the method and then use forEach, everything is fine. The second answer here mentions exactly what you are trying to do.

Answer 2

There is little to no reason to collect a entirely new List if you don't mutate it at all. Besides that your use case is basically iterating over every element in a collection and save that which could simply be achieved by using a for-each.

If for some reason thingRepo.save(thing) mutates the object you can still return the same collection, but at this point it's a hidden mutation which is not clearly visible at all since thingRepo.save(thing) does not suggest that.