CODEWITHSUNDEEP
javacollectionsforeachjava-8java-stream
user1796346
user1796346

Reputation: 109

Java 8 - Streams Nested ForEach with different Collection

I try to understand the new Java 8 Streams and I tried for days to transfer nested foreach loops over collection in Java 8 Streams.

Is it possible to refactor the following nested foreach loops including the if-conditions in Java-8-Streams?

If yes what would it look like.

ArrayList<ClassInq> Inq = new ArrayList<>();
TreeMap<String, SalesQuot> Quotations = new TreeMap<>();

ArrayList<ClassInq> tempInqAndQuot = new ArrayList<>();
ArrayList<SalesQuot> tempQuotPos = new ArrayList<>();   

for(ClassInq simInq : this.Inq) {
    if(!simInq.isClosed() && !simInq.isDenied()) {
        for(Map.Entry<String, SalesQuot> Quot: Quotations.entrySet()) {

            SalesQuot sapQuot = Quot.getValue();

            if(sapQuot.getInquiryDocumentNumber().compareTo(simInq.getSapInquiryNumber()) == 0) {

                simInq.setSAPQuotationNumber(sapQuot.getQuotationDocumentNumber());
                tempInqAndQuot.add(simInq);

                for(Map.Entry<String, SalesQuotPosition> quotp : sapQuot.getPosition().entrySet()) {
                    tempQuotPos.add(quotp.getValue());
                }
            }
        }
    }
}

Thanks a lot for your help.

BR

Upvotes: 3

Views: 26018

Answers (1)

Holger
Holger

Reputation: 298579

First, try to adhere to the Java naming conventions, as your upper case variable names make it really hard to read your code. Second, it’s a good thing that you want to learn about Stream API but you should not ignore the basics of the pre-Java 8 Collection APIs.

It’s not useful to iterate over an entrySet() when you are only interested in either, keys or values. You do it two times within a small piece of code.

At the first appearance you can replace

for (Map.Entry<String, SalesQuot> Quot: Quotations.entrySet()){
    SalesQuot sapQuot = Quot.getValue();

with the simpler

for (SalesQuot sapQuot: Quotations.values()){

At the second, the entire

for(Map.Entry<String,SalesQuotPosition> quotp: sapQuot.getPosition().entrySet()){
    tempQuotPos.add(quotp.getValue());
}

can be replaced by

tempQuotPos.addAll(sapQuot.getPosition().values());

Thus even without streams, your code can be simplified to

for (ClassInq simInq : this.Inq){
        if (!simInq.isClosed() && !simInq.isDenied()){      
            for (SalesQuot sapQuot: Quotations.values()){
                if (sapQuot.getInquiryDocumentNumber().compareTo(simInq.getSapInquiryNumber()) == 0){
                    simInq.setSAPQuotationNumber(sapQuot.getQuotationDocumentNumber());
                    tempInqAndQuot.add(simInq);
                    tempQuotPos.addAll(sapQuot.getPosition().values());
                }
            }
        }
    }

though it’s still not clear what it is supposed to do and whether it’s correct. Besides the errors and suspicions named in the comments to your question, modifying the incoming values (esp. from the outer loop) does not look right.

It’s also not clear why you are using ….compareTo(…)==0 rather than equals.

However, it can be straight-forwardly rewritten to use streams without changing any of the code’s logic:

this.Inq.stream().filter(simInq -> !simInq.isClosed() && !simInq.isDenied())
  .forEach(simInq -> Quotations.values().stream().filter(sapQuot ->
   sapQuot.getInquiryDocumentNumber().compareTo(simInq.getSapInquiryNumber())==0)
   .forEach(sapQuot -> {
      simInq.setSAPQuotationNumber(sapQuot.getQuotationDocumentNumber());
      tempInqAndQuot.add(simInq);
      tempQuotPos.addAll(sapQuot.getPosition().values());
    })
  );

Still, I recommend cleaning up the original logic first before rewriting it for using other APIs. The stream form would greatly benefit from a more precise definition of what to achieve.

Upvotes: 13

Related Questions