How could I simplify this with Java stream library?

Question

for (Issue issue : issues) {
     if (issue.getSubtasks().spliterator().estimateSize() > 0) {
         alreadyCreated = false;
         for (Subtask subtask : issue.getSubtasks()) {
            if (subtask.getSummary().contains("sometext")) {
                alreadyCreated = true;
                break;
            }
         }
         if (!alreadyCreated) {
            ids.add(issue.getKey());
         }
    } else {
         ids.add(issue.getKey());
    }
}

I'm not so expert with Java stream API, but I'm pretty sure there's some way to simplify the code above with using lambda expressions. Would be a great help to understand it even better!

Some notes: Issues and getSubtasks() are returning back Iterable<> types.

What type data structure is issues? And what does getSubTasks() return? And providing a little more written detail as to what you are trying to do and showing some classes would help. — WJS, Feb 23 '21 at 19:33
looks like you are adding `key` into `ids` arraylist at any time — Ryuzaki L, Feb 23 '21 at 19:40
@Deadpool I don't think so. If the size > 0 and `alreadyCreated` is `true` then it doesn't add it. The indentation needs to be improved. — WJS, Feb 23 '21 at 19:42
Yeah. But I still have questions about this in terms of data types, etc. — WJS, Feb 23 '21 at 19:44
what's the point of `(issue.getSubtasks().spliterator().estimateSize() > 0)` ? I'm asking because after that check, you iterate over all subtasks of the issue from the begging, although it can be legit, but potential indicator then something could be optimised. — SergeiTonoian, Feb 23 '21 at 19:50

Unmitigated · Accepted Answer · 2021-02-24T16:49:58.430

1

You can use filter to remove unwanted elements and map to get the key, then collect to a List.

StreamSupport.stream(issues.spliterator(), false).filter(x -> 
   StreamSupport.stream(x.getSubtasks().spliterator(), false)
     .noneMatch(y -> y.getSummary().contains("sometext"))
).map(Issue::getKey).collect(Collectors.toList());

edited Feb 24 '21 at 16:49

answered Feb 23 '21 at 22:10

Unmitigated

76,500
11
62
80

Aren't you guessing? How do you know `getSubtasks()` is streamable? It could return any class that implemented the `Spliterator` interface. It doesn't have to be a stream. – WJS Feb 23 '21 at 22:16
1

I suppose it's possible that `getSubtasks()` isn't a plain old collection. It's even possible that `issues` isn't streamable. But OP is specifically asking about the stream API, so it seems reasonable to assume that things are streamable. – Willis Blackburn Feb 23 '21 at 22:40
The only problem the issues type is "Iterable issues" so it's not streamable by default, same as getSubtasks :/ – Dava Loper Feb 24 '21 at 08:17
2

@DavaLoper see [Convert Iterable to Stream](https://stackoverflow.com/q/23932061/2711488) – Holger Feb 24 '21 at 09:04
Yes I saw this, but it makes a bit ugly if I replace everywhere... – Dava Loper Feb 24 '21 at 09:32
2

What is stopping you from putting this code into a utility method? – Holger Feb 24 '21 at 09:42

WJS · Answer 2 · 2021-02-24T20:47:32.063

I can't be certain what is streamable in your example so I'm going to provide an alternate solution that doesn't require streams but is at least, if not more, efficient. It just uses a different technique but essentially the same logic.

if size <= 0, then the for loop is skipped and the key is added.
if size > 0 then then for loop is excecuted. Then if any of the summaries contains the text, the outer loop proceeds normally, otherwise, the loop falls thru ant the key is added.

outer:
for (Issue issue : issues) {
     if (issue.getSubtasks().spliterator().estimateSize() > 0) {
         for (Subtask subtask : issue.getSubtasks()) {
            if (subtask.getSummary().contains("sometext")) {
                continue outer;          
            }               
         }
     } 
     ids.add(issue.getKey());     
    }
}

Thanks, you were right it's not streamable, so actually this was a great solution, thanks! — Dava Loper, Feb 24 '21 at 08:18

Willis Blackburn · Answer 3 · 2021-02-24T14:47:05.670

0

You're adding the issue key to ids if there exists no subtask with a summary containing "sometext."

issues.stream()
    .filter(i -> i.getSubtasks().stream()
        .noneMatch(s -> s.getSummary().contains("sometext")))
    .map(Issue::getKey)
    .forEach(ids::add);

I think the subtasks size check is at least superfluous and possibly dangerous, if it's possible for it to estimate the size as zero when subtasks exist. But if getSubtasks() returns some non-collection that can't easily be streamed and this estimateSize() call is necessary then that just changes things a little bit:

issues.stream()
    .filter(i -> {
        Spliterator<Subtask> split = i.getSubtasks().spliterator();
        return split.estimateSize() == 0 ||
            StreamSupport.stream(split, false)
               .noneMatch(s -> s.getSummary().contains("sometext"));
    })
    .map(Issue::getKey)
    .forEach(ids::add);

edited Feb 24 '21 at 14:47

answered Feb 23 '21 at 20:51

Willis Blackburn

8,068
19
36

The only problem the issues type is "Iterable issues" so it's not streamable by default, same as getSubtasks :/ – Dava Loper Feb 24 '21 at 08:16
2

I’d remove the `split.estimateSize() == 0 ||`. It’s a questionable premature optimization, especially when taking an *estimate* size for granted. – Holger Feb 24 '21 at 09:50
You can use the strategy in the second example to make `issues` into a `Stream`, just call `spliterator()` on it and then use `StreamSupport.stream()`. – Willis Blackburn Feb 24 '21 at 14:48

How could I simplify this with Java stream library?

3 Answers3