Alternative to multiple nested for and if blocks

Question

I'm trying to implement a complicated block that I've written using multiple for loops and if conditions to something much less convoluted. The initial code is

for(Coll_Monthly_Main monthlyAccount:monthlyList){
        for(Coll_Daily_Main dailyAccount:dailyList){
            if(monthlyAccount.getAccountId().trim().equals(dailyAccount.getAccountId().trim())){
                for(Catg_Monthly monthlyCategory: monthlyAccount.getCatg()){
                    for(Catg_Daily dailyCategory: dailyAccount.getCatg()){
                        if(monthlyCategory.getPriCatId().trim().equals(dailyCategory.getPriCatId().trim())){
                            monthlyCategory.setMthTtl(dailyCategory.getMthTtl());
                            monthlyCategory.setMtd(dailyCategory.getMtd());
                            monthlyCategory.setYtd(dailyCategory.getYtd());

                            for(SecCatDtl_Mthly monthlySecCategory:monthlyCategory.getSecCatDtl()){
                                for(SecCatDtl_Daily dailySecCategory:dailyCategory.getSecCatDtl()){
                                    if(monthlySecCategory.getCatId().trim().equals(dailySecCategory.getCatId().trim())){
                                        monthlySecCategory.setMthTtl(dailySecCategory.getMthTtl());
                                        monthlySecCategory.setMtd(dailySecCategory.getMtd());
                                        monthlySecCategory.setYtd(dailySecCategory.getYtd());
                                    }
                                }
                            }
                        }

                    }
                }
            }
        }
    }
    return monthlyList;

I've followed this answer and have managed to implement the first level as below:-

monthlyList.forEach(coll_mthly->{
        dailyList.stream().filter(coll_daily->coll_mthly.getAccountId().trim().equals(coll_daily.getAccountId().trim()))
        .forEach(catg_mth->coll_mthly.getCatg())->{

        };
    });

For the next level of nesting, I need to loop over a nested list and I'm not sure how to proceed about it. I keep getting a syntax error as follows:- Syntax error on tokens, TypeElidedFormalParameter expected instead

I'd appreciate any pointers in the right direction.

Update:- This is how it looks like following Thomas' answer

Map<String, Coll_Daily_Main> dailies = dailyList.stream().collect(Collectors.toMap(cdm -> cdm.getAccountId(), cdm-> cdm) );

    for(Coll_Monthly_Main monthlyAccount : monthlyList) {
        Coll_Daily_Main dailiesForAccount  = dailies.get( monthlyAccount.getAccountId().trim());

          Map<String, Catg_Daily> dailyCatgories=dailiesForAccount.getCatg().stream().collect(Collectors.toMap(cv->cv.getPriCatId(), cv->cv));
          for(Catg_Monthly monthlyCategory:monthlyAccount.getCatg()){
              Catg_Daily dailyCategory = dailyCatgories.get(monthlyCategory.getPriCatId().trim());
              if(dailyCategory!=null){
                monthlyCategory.setMthTtl(dailyCategory.getMthTtl());
                monthlyCategory.setMtd(dailyCategory.getMtd());
                monthlyCategory.setYtd(dailyCategory.getYtd());

                Map<String,SecCatDtl_Daily> dailySecCategories=dailyCategory.getSecCatDtl().stream().collect(Collectors.toMap(fg->fg.getCatId(), fg->fg));
                for(SecCatDtl_Mthly monthlySecCategory:monthlyCategory.getSecCatDtl()){
                    SecCatDtl_Daily dailySecCategory =dailySecCategories.get(monthlySecCategory.getCatId().trim());
                    if(dailySecCategory!=null){
                    monthlySecCategory.setMthTtl(dailySecCategory.getMthTtl());
                    monthlySecCategory.setMtd(dailySecCategory.getMtd());
                    monthlySecCategory.setYtd(dailySecCategory.getYtd());
                    }
                }
              }
          }
        }

Answer 1

As the others already stated multiple times it's most likely better to rethink your approach and make it not even more readable but faster as well. One thing that comes to my mind: you have 3 levels that consist of 2 loops and an if to check whether the elements match (by id). Those levels will have O(n*m) complexity.

However, you could try to build a map or multimap (Guava has some) with the id as the key and get it down to O(n + m):

• O(n) for building the map (ideally on on the larger set, i.e. daily)
• O(m) for iterating over the second set (ideally the smaller set, i.e. monthly)
• Lookups should be O(1) so can be ignored

I'm not sure what all those nested levels mean so I only can give an example of what you could do for one level (I'll take the first):

//I'm using Google Guava's classes here
SetMultimap<String, Coll_Daily_Main> dailies = ...;//use one that fits your needs

//Iterate over n daily entries and put them into the map which should be O(n)
dailyList.forEach( cdm -> dailies.put( cdm.getAccountId().trim(), cdm ) );

//Iterate over the (smaller) set of monthly entries and do a lookup for the dailies which should be O(m)
for(Coll_Monthly_Main monthlyAccount : monthlyList) {
  Set<Coll_Daily_Main> dailiesForAccount = dailies.get( monthlyAccount.getAccountId().trim() );

  //level done, either go down to the next or try to further straighten it out or optimize
}

Update:

I forgot to mention that you'd not have to use Guava with Java 8. Though the definition looks a little more awkward using a Map<String, Set<Coll_Daily_Main>> isn't that "hard" anymore:

 Map<String, Set<String>> dailies = new HashMap<>();
 dailyList.forEach( cdm -> dailies.computeIfAbsent( cdm.getAccountId().trim(), v -> new HashSet<>() ).add( cdm  ) );

Note: you could also use collectors to make it a little shorter and in one line. Whether that's easier to read and use is up to debate.

Map<String, Set<Daily>> dailies = 
    dailyList.stream().collect( Collectors.groupingBy( cdm -> cdm.getAccountId().trim(), 
                                                       Collectors.toSet() ) );