CODEWITHSUNDEEP
javafor-loopjava-8java-stream
deepakP
deepakP

Reputation: 13

Converting nested for loop from Java 7 to Java 8 using Streams

I have following nested for loops which I need to convert to JAVA 8 stream API format 

for (EmpInfo empInfo : empInfos) {

  for (AddressInfo address : addressInfos) {

    if (address.getStreetNumber() != null 
      && empInfo.getStreetNumber() != null 
      && address.getStreetNumber().longValue() == empInfo.getStreetNumber().longValue()) {

      empInfo.setStreetName(address.getStreetName());
      empInfo.setStreetZipCode(address.getStreetZipCode());
    }
  }
}

Any suggestions?

Upvotes: 1

Views: 1445

Answers (2)

RubioRic
RubioRic

Reputation: 2468

Well, I don't like nested loops very much ... Is it mandatory to create a one-liner stream operation? What about splitting the code a bit to improve readability?

Let's assume that streetNumber is a key assigned to a pair streetName/zipCode and that you're trying to copy that information from AddressInfo to EmpInfo using such field as "matching column".

I've programmed a little functional example using these classes

EmpInfo

public class EmpInfo {

      private String id;

      private Long streetNumber;

      private String streetName;

      private String streetZipCode;

      /** get/set methods */

      @Override
      public String toString() {
           return "EmpInfo [id=" + id + ", streetNumber=" + streetNumber 
                + ", streetName=" + streetName + ", streetZipCode=" + streetZipCode + "]";
      }
}

AddressInfo

public class AddressInfo {

      private String id;

      private Long streetNumber;

      private String streetName;

      private String streetZipCode;

      /** get/set methods */
}

TestAddress

public class TestAddress {

public static void main(String[] args) {

    List<EmpInfo> listEmp = new ArrayList<>();

    EmpInfo emp1 = new EmpInfo("Fail1", null, null, null);
    EmpInfo emp2 = new EmpInfo("Match1", Long.valueOf(1), null, null);
    EmpInfo emp3 = new EmpInfo("Match2", Long.valueOf(1), "streeNameEmp", "zipCodeEmp");
    EmpInfo emp4 = new EmpInfo("Match3", Long.valueOf(2), "streeNameEmp", "zipCodeEmp");
    EmpInfo emp5 = new EmpInfo("Fail3",  Long.valueOf(3), "streeNameEmp", "zipCodeEmp");

    listEmp.add(emp1);
    listEmp.add(emp2);
    listEmp.add(emp3);
    listEmp.add(emp4);
    listEmp.add(emp5);

    List<AddressInfo> listAddress = new ArrayList<>();

    AddressInfo add1 = new AddressInfo("Address1", Long.valueOf(1), "Street One", "Zip One");
    AddressInfo add2 = new AddressInfo("Address2", Long.valueOf(2), "Street Two", "Zip Two");
    AddressInfo add4 = new AddressInfo("Address4", Long.valueOf(4), "Street Three", "Zip Four");
    AddressInfo add5 = new AddressInfo("Address5", null, "Street Three", "Zip Four");

    listAddress.add(add1);
    listAddress.add(add2);
    listAddress.add(add4);
    listAddress.add(add5);

    Map<Long, AddressInfo> mapAddresses = 
            listAddress.stream().filter(address -> address.getStreetNumber() != null)
                                .collect(Collectors.toMap(AddressInfo::getStreetNumber, 
                                                          Function.identity()));

    listEmp.stream().filter(emp -> mapAddresses.containsKey(emp.getStreetNumber()))
                    .forEach(emp -> copyAddressData2Emp(mapAddresses, emp));

    listEmp.stream().forEach(emp -> System.out.println(emp));
}

private static void copyAddressData2Emp(Map<Long, AddressInfo> mapAddresses, EmpInfo emp) {
    AddressInfo addressInfo = mapAddresses.get(emp.getStreetNumber());
    emp.setStreetName(addressInfo.getStreetName());
    emp.setStreetZipCode(addressInfo.getStreetZipCode());
}

}

What I have done? First I've created a Map with the AddressInfo objects using field streetNumber as key. Those AddressInfo whose streetNumber is null have been previously filtered.

 Map<Long, AddressInfo> mapAddresses = 
     listAddress.stream().filter(address -> address.getStreetNumber() != null)
                         .collect(Collectors.toMap(AddressInfo::getStreetNumber, 
                                                   Function.identity()));

Then after filtering those EmpInfo who match an AddressInfo, the information is copied from the matching AddressInfo object to the EmpInfo object. Notice that EmpInfo with null values in streetNumber are filtered from the stream without without explicitly forcing it.

 listEmp.stream().filter(emp -> mapAddresses.containsKey(emp.getStreetNumber()))
                 .forEach(emp -> copyAddressData2Emp(mapAddresses, emp));

Probably you can simplify this stream operation, some way of passing the matching AddressInfo to the following operation in the stream, but I've not found a simpler option. Suggestions are welcome.

Upvotes: 0

Jordan
Jordan

Reputation: 2283

This is a great example to show why it's not always a good idea to convert something to streams just because you can. Here's a fairly compact attempt at streamifying this code:

empInfos.stream()
    .filter(empInfo -> empInfo.getStreetNumber() != null)
    .forEach(empInfo -> addressInfos.stream()
        .filter(addressInfo -> addressInfo.getStreetNumber() != null && addressInfo.getStreetNumber().longValue() == empInfo.getStreetNumber().longValue())
        .forEach(addressInfo -> {
          empInfo.setStreetName(addressInfo.getStreetName());
          empInfo.setStreetZipCode(addressInfo.getStreetZipCode());
        }));

Now, let's be honest, that's not as easy to read as the code you started with. A couple of nested loops are very straightforward. And more importantly, they're easy to debug. This monstrosity is going to be a nightmare to do any debugging with. Setting breakpoints inside of lamdas can be a challenge.

My advice to you: Keep your code as-is. Nested loops aren't sexy, but they're reliable, easy to understand, and easy to debug. And it's not like you're going to get any significant performance boost out of the stream version of this code.

Upvotes: 5

Related Questions