3

I converted the following code into java 8 code. I would like to know if i did it properly or there is some other nice way.

Java 7

for (final Category category : categories) { final List<Category> subCategories = getCategories(category); if (subCategories != null) { currentLevel.addAll(subCategories); } }

Java8

categories.stream().map(category -> getCategories(category)).filter(list->list!=null).flatMap(cat -> cat.parallelStream()).collect(Collectors.toList())

Any java 8 way to solve the following code into a compact form.

while (CollectionUtils.isNotEmpty(currentLevel)) { for (final Iterator<Category> iterator = currentLevel.iterator(); iterator.hasNext();) { final Category category = iterator.next(); if (result == null) { result = new HashSet<Category>(); } if (!result.add(category)) { // avoid cycles by removing all which are already found iterator.remove(); } } if (currentLevel.isEmpty()) { break; } final Collection<Category> nextLevel = getAllSubcategories(currentLevel); currentLevel = nextLevel; }
Improve this question
7
  • For me it looks quite good. Commented Jun 8, 2016 at 20:37
  • Updated ...i missed one clause in the beginning .flatMap(cat -> cat.parallelStream()) Commented Jun 8, 2016 at 20:38
  • 1
    Are you just trying to end up with unique categories? If so you might just use @stholzm's solution but use Collectors.toSet(). Or, if you need a list, add a .distinct() step after the flatMap Commented Jun 9, 2016 at 3:11
  • Do you really need a 3rd party library for invoking currentLevel.isEmpty()? Commented Jun 9, 2016 at 8:35
  • @Holger Apache Commons CollectionUtils.isNotEmpty and related methods are "null-safe" in that a null collection is considered empty. Not my preference, but a lot of people like this style. Commented Jun 10, 2016 at 0:09

3 Answers 3

Reset to default
5

Your solution is ok except the fact that flat-mapping to parallel stream is useless. If you take a look into flatMap implementation in OpenJDK/OracleJDK, you can see that the stream created via lambda passed into flatMap is immediately turned into sequential mode. So you will not have any parallelism and it's better to replace parallelStream() with stream() to avoid confusion. If you really want to parallelize the work, it's usually good idea to parallelize the outermost stream only.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

4

Maybe a matter of preference, but you could replace those lambdas with method references:

categories.stream() .map(this::getCategories) .filter(Objects::nonNull) .flatMap(List::stream) .collect(Collectors.toList())

Too bad the lists can be null apparently (really?)... otherwise you could just use flatMap.

And I wouldn't use parallelStream in another stream operation.

Improve this answer

4 Comments

Looks very elegant. +1 . I will try to remove the null condition. How can i do with iterator like i updated.
If you're using an Iterator, it's not going to be much better. Why do you want to do it with an iterator? That's like asking "how can I build a sand castle with this pair of tweezers." You can do it, but it's going to be longer and harder than using the right tools for the job.
@LouisWasserman : Oh iterator was the second question i was referring to. Sorry for the confusion.
It doesn't look like that's possible to do with streams. You can't e.g. do anything like the iterator.remove() with streams.
2

For the second question, the usual streams-based approach to removing duplicates is to copy the elements into a different collection, processing them using the distinct() operation:

 Collection<Category> currentCopy = currentLevel.stream() .distinct() .collect(toList());

But it seems that you're trying to manipulate the collection in-place instead of making a copy. To do this, you could do something like the following:

 Set<Category> result = new HashSet<>(); currentLevel.removeIf(cat -> !result.add(cat));

Note that this isn't a streams operation, so you can't run it in parallel. You can't get much parallelism anyway, since the predicate has side effects.

Improve this answer

2 Comments

What do you mean by "since the predicate has side effects" ?
@Jean-FrançoisSavard A pure function produces its result only by reading values from its input and performing computations on those values. An impure function, or one with side effects, does things other than computing a result from its input. In my example the predicate cat -> !result.add(cat) mutates result. That's a side effect. If this were converted to a parallel stream, then multiple threads would be mutating result simultaneously, which would lead to corruption since HashSet isn't thread safe. That in turn would require synchronization or use of ConcurrentHashMap.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.