5

I have the following code: (simplified for bravity)

public void search(Predicate<String> predicate, Elements elements)
{
    List<SearchResult> searchResults = elements.stream()
        .filter(element -> predicate.test(element.ownText()))
        .map(element -> new SearchResult(element.ownText(), element.baseUri(),element.tagName()))
        .collect(Collectors.toList());
}

But now, I want to have another list which would contain all filtered elements without the mapping. Is it possible to do that with a stream, or should I change to a foreach loop for that?

Henrik Aasted Sørensen
  • 6,966
  • 11
  • 51
  • 60
Mr.WorshipMe
  • 713
  • 4
  • 16
  • 1
    You could do it with a custom collector. But another solution would be to have 2 Stream pipeline. – Tunaki Dec 07 '15 at 13:06

4 Answers4

8

You can collect the elements after filtering and then use this list to map the elements to search results:

public void search(Predicate<String> predicate, Elements elements) {
    List<Element> filteredElements = 
        elements.stream()
                .filter(element -> predicate.test(element.ownText()))
                .collect(Collectors.toList());

    List<SearchResult> searchResults = 
        filteredElements.stream()
                        .map(element -> new SearchResult(element.ownText(),element.baseUri(),element.tagName()))
                        .collect(Collectors.toList());
}

This won't take more time than the solutions of the other answers but doesn't have side effects, is thread safe as well as easy to read and understand.

Acanda
  • 672
  • 5
  • 12
5

A simple way to do this would be

List<Element> filteredElements = new ArrayList<>();
List<SearchResult> searchResults = elements.stream()
      .filter(element -> predicate.test(element.ownText()))
      .peek(filteredElements::add)
      .map(element -> 
         new SearchResult(element.ownText(),element.baseUri(),element.tagName()))
      .collect(Collectors.toList());
Louis Wasserman
  • 191,574
  • 25
  • 345
  • 413
  • Nice and elegant! Thank you. – Mr.WorshipMe Dec 07 '15 at 21:26
  • 4
    This use of peek gives me a very bad feeling, although I cannot state anything specific that is wrong with it. It feels like an abuse of peek. – David Conrad Dec 07 '15 at 23:38
  • 3
    @DavidConrad because from the [`peek` Javadoc](https://docs.oracle.com/javase/8/docs/api/java/util/stream/Stream.html#peek-java.util.function.Consumer-): “_This method exists mainly to support debugging, where you want to see the elements as they flow past a certain point in a pipeline_”. – Didier L Dec 08 '15 at 12:27
  • 1
    @DavidConrad your senses are probably tingling because `peek(filteredElements::add)` introduces a side effect (adding elements to `filteredElements`). Also, when `elements.stream()` returns a parallel stream, `filteredElements` may not contain all filtered elements as ArrayList is not thread safe. – Acanda Jul 15 '16 at 15:04
  • @Acanda Good call. It didn't take much to get `ArrayList` to break. A straightforward parallel use results in either missing elements or an `ArrayIndexOutOfBoundsException`. A `ConcurrentLinkedQueue` avoids that problem, though I'm still not thrilled with this usage. – David Conrad Jul 15 '16 at 16:10
  • 1
    @DavidConrad I added an answer with a solution that's both thread safe and without side effects. – Acanda Jul 15 '16 at 16:24
2

One alternative is to define a Supplier<Stream>:

public void search(Predicate<String> predicate, Elements elements)
{
    Supplier<Stream<Element>> supplier = () -> elements.stream()
           .filter(element -> predicate.test(element.ownText()));
    List<SearchResult> searchResults = supplier.get()
           .map(element -> new SearchResult(element.ownText(),element.baseUri(),element.tagName()))
           .collect(Collectors.toList());
    List<Element> elementList = supplier.get().collect(Collectors.toList());
}

Note that using this approach you actually perform the filtering twice.

An alternative (though not very beautiful in this case) solution is use pairing collector from this answer:

Collector<Element, ?, List<SearchResult>> c1 = 
    Collectors.mapping(element -> new SearchResult(element.ownText(),element.baseUri(),element.tagName()),
        Collectors.toList());
Collector<Element, ?, List<Element>> c2 = Collectors.toList();
Collector<Element, ?, Pair<List<SearchResult>, List<Element>>> pairCollector =
    pairing(c1, c2, Pair::new); // Assumes some Pair class existing

Pair<List<SearchResult>, List<Element>> result = elements.stream()
           .filter(element -> predicate.test(element.ownText()))
           .collect(pairing);

These solutions are generic: they allow to do two different operations with single data source. But in your concrete example it's easier to create first list of filtered non-mapped data, then create a second stream on that list:

List<Element> elementList = elements.stream()
           .filter(element -> predicate.test(element.ownText()))
           .collect(Collectors.toList());
List<SearchResult> searchResults = elementList.stream()
           .map(element -> new SearchResult(element.ownText(),element.baseUri(),element.tagName()))
           .collect(Collectors.toList());
Community
  • 1
  • 1
Tagir Valeev
  • 97,161
  • 19
  • 222
  • 334
  • I think the last approach is the best in this case. However there are actually 3 very different answers here, maybe worth splitting them? – Didier L Dec 08 '15 at 12:44
0

You cannot use the stream twice, as it is closed when using collect. The following code will throw an "java.lang.IllegalStateException: stream has already been operated upon or closed". So you need to create the stream twice or use a for-each loop.

import java.util.Collections;
import java.util.List;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;

class Test
{
    static class SearchResult
    {
        SearchResult(Object o,Object p,Object q) {}
    }

    static class Element
    {
        public String ownText() {return "text";}
        public String tagName() {return "tag";}
        public String baseUri() {return "base";}
    }

    public static void search(Predicate<String> predicate, List<Element> elements)
    {
        Stream<Element> stream = elements.stream().filter(element -> predicate.test(element.ownText()));
        List<Element> filtered = stream.collect(Collectors.toList());
        // next line will throw the IllegalStateException as above line has already consumed the stream
        List<SearchResult> results =    stream.map(element -> 
         new SearchResult(element.ownText(),element.baseUri(),element.tagName()))
                .collect(Collectors.toList());
    }

    public static void main(String[] args)
    {       
        search(s->true,Collections.singletonList(new Element()));
    }
}
Didier L
  • 18,905
  • 10
  • 61
  • 103
Konrad Höffner
  • 11,100
  • 16
  • 60
  • 118