2

I have this piece of code and I want to return a list of postCodes:

List<String> postcodes = new ArrayList<>();
List<Entry> entries = x.getEntry(); //getEntry() returns a list of Entry class
for (Entry entry : entries) {
    if (entry != null) {
       Properties properties = entry.getContent().getProperties();
       postcodes.addAll(Arrays.asList(properties.getPostcodes().split(",")));
   }
} 
return postcodes;

Here's my attempt to use stream() method and the following chained methods:

...some other block of code
List<Entry> entries = x.getEntry.stream()
    .filter(entry -> recordEntry != null)
    .flatMap(entry -> {
        Properties properties = recordEntry.getContent().getProperties();
        postCodes.addAll(Arrays.asList(properties.getPostcodes().split(",")));
});
invincibles04
  • 109
  • 3
  • 13

1 Answers1

5

you've got several issues with your code i.e:

  1. postCodes.addAll is a side-effect and therefore you should avoid doing that otherwise when the code is executed in parallel you'll receive non-deterministic results.
  2. flatMap expects a stream, not a boolean; which is what your code currently attempts to pass to flatMap.
  3. flatMap in this case consumes a function that also consumes a value and returns a value back and considering you've decide to use a lambda statement block then you must include a return statement within the lambda statement block specifying the value to return. this is not the case within your code.
  4. stream pipelines are driven by terminal operations which are operations that turn a stream into a non-stream value and your code currently will not execute at all as you've just set up the ingredients but not actually asked for a result from the stream.
  5. the receiver type of your query should be List<String> not List<Entry> as within your current code the call to Arrays.asList(properties.getPostcodes().split(",")) returns a List<String> which you then add to an accumulator with the call addAll.
  6. thanks to Holger for pointing it out, you're constantly failing to decide whether the variable is named entry or recordEntry.

That said here's how I'd rewrite your code:

List<String> entries = x.getEntry.stream()
        .filter(Objects::nonNull)
        .map(Entry::getContent)
        .map(Content::getProperties)
        .map(Properties::getPostcodes‌)
        .flatMap(Pattern.co‌mpile(",")::splitAsS‌tream)
        .collect(Collectors.toList());

and you may want to use Collectors.toCollection to specify a specific implementation of the list returned if deemed appropriate.

edit:

with a couple of good suggestions from shmosel we can actually use method references throughout the stream pipelines and therefore enabling better intent of the code and a lot easier to follow.

or you could proceed with the approach:

List<String> entries = x.getEntry.stream()
       .filter(e -> e != null)
       .flatMap(e -> Arrays.asList(
         e.getContent().getProperties().getPostcodes().split(",")).stream()
       )
       .collect(Collectors.toList());

if it's more comfortable to you.

Ousmane D.
  • 54,915
  • 8
  • 91
  • 126
  • 3
    You can replace `Arrays.asList(...).stream()` with `Arrays.stream(...)` or `Stream.of(...)`. – shmosel Dec 19 '17 at 22:57
  • @shmosel true, very much appreciated for the suggestion. – Ousmane D. Dec 19 '17 at 22:58
  • 2
    If you prefer method references, you can also do `.map(Entry::getContent).map(Content::getProperties).map(Properties::getPostcodes).flatMap(Pattern.compile(",")::splitAsStream)`. – shmosel Dec 19 '17 at 23:00
  • @shmosel once again very much appreciated. :) – Ousmane D. Dec 19 '17 at 23:03
  • 2
    You don’t need `Arrays.asList(…).stream()`; use `Arrays.stream(…)` to get a stream in the first place. And the list of issues is incomplete. Another issue of the original code is that the OP constantly fails to decide whether the variable is named `entry` or `recordEntry`. – Holger Dec 20 '17 at 08:53
  • @Holger I've intentionally left it as `Arrays.asList(…).stream()` to keep it consistent with OPs current approach, though as always you're correct and it would be better to use `Arrays.stream(…)`. regarding your second point; I did see the issue with OP failing to decide on the variable name `entry` or `recordEntry` but for some reason decided to not mention it (which I should have done). very much appreciated once again. – Ousmane D. Dec 20 '17 at 13:06