CODEWITHSUNDEEP
javalambdajava-8java-streamrefactoring
KItis
KItis

Reputation: 5646

Refactoring Java For loop code to use Java 8 streams API

I have following method which is used for creating a order in the database, order has many items and, item has many bills. iPadPOSOrderDTO is the order which is going to base saved into the database.

so, the loop based code for creating order is the following

 private void createNewOrder(IPadPOSOrderDTO iPadPOSOrderDTO) {
    IPadPOSOrderV2 order = mapper.map(iPadPOSOrderDTO, IPadPOSOrderV2.class);
    if(order.getOrderV2Bills()!=null && order.getOrderV2Bills().size()>0){
        for(IPadPOSOrderV2Bill orderBill : order.getOrderV2Bills()){
            orderBill.setOrder(order);

            if(orderBill.getiPadPOSOrderV2BillItems()!=null && orderBill.getiPadPOSOrderV2BillItems().size()>0){
                for(IPadPOSOrderV2BillItems orderBillItem :  orderBill.getiPadPOSOrderV2BillItems()){
                    orderBillItem.setiPadPOSOrderV2Bill(orderBill);
                    orderBillItem.setOrderId(order.getOrderId());

                }
            }
        }
    }

    sessionFactory.
            getCurrentSession().save(order);
}

I wanted to refactor above code to use Java 8 streams API.

So, I did the following 

private void createNewOrderV2(IPadPOSOrderDTO iPadPOSOrderDTO) {
        IPadPOSOrderV2 order = mapper.map(iPadPOSOrderDTO, IPadPOSOrderV2.class);
        if(order.getOrderV2Bills()!=null && order.getOrderV2Bills().size()>0){
            order.getOrderV2Bills().stream().forEach(e -> { createBill(order,e);});
        }
        sessionFactory.
                getCurrentSession().save(order);
    }

    private void createBill(IPadPOSOrderV2 ipadExistingOrderFromDatabase, IPadPOSOrderV2Bill iPadPOSOrderV2Bill) {
        iPadPOSOrderV2Bill.setOrder(ipadExistingOrderFromDatabase);

        if(iPadPOSOrderV2Bill.getiPadPOSOrderV2BillItems()!=null && iPadPOSOrderV2Bill.getiPadPOSOrderV2BillItems().size()>0){
            iPadPOSOrderV2Bill.getiPadPOSOrderV2BillItems().stream().forEach(e -> createBillItem(ipadExistingOrderFromDatabase,iPadPOSOrderV2Bill,e));
        }
    }

    private void createBillItem(IPadPOSOrderV2 ipadExistingOrderFromDatabase, IPadPOSOrderV2Bill iPadPOSOrderV2Bill, IPadPOSOrderV2BillItems iPadPOSOrderV2BillItem) {
        iPadPOSOrderV2BillItem.setiPadPOSOrderV2Bill(iPadPOSOrderV2Bill);
        iPadPOSOrderV2BillItem.setOrderId(ipadExistingOrderFromDatabase.getOrderId());
        ipadExistingOrderFromDatabase.getOrderV2Bills().stream().forEach(e -> { createBill(ipadExistingOrderFromDatabase,e);});
    }

could somebody share their experience and advice me if I am making the correct use of streams API for this refactoring.

Upvotes: 0

Views: 316

Answers (2)

Thomas
Thomas

Reputation: 88707

Note that those size checks aren't really necessary. An empty list would result in an empty stream and thus nothing would get applied. The only benefit would be that you'd be able to avoid having to create the stream altogether but I highly doubt the performance difference would even be noticeble.

If you want to convert a potentially null collection to a stream you might want to use a small helper function:

public <T> Stream<T> collectionToStream(Collection<T> collection) {
  return Optional.ofNullable(collection).map(Collection::stream).orElseGet(Stream::empty);
}

Using forEach() you could then do something like this:

private void createNewOrder(IPadPOSOrderDTO iPadPOSOrderDTO) {
  IPadPOSOrderV2 order = mapper.map(iPadPOSOrderDTO, IPadPOSOrderV2.class);
  collectionToStream(order.getOrderV2Bills()).forEach( orderBill -> {
      orderBill.setOrder(order);

      collectionToStream(orderBill.getiPadPOSOrderV2BillItems()).forEach(orderBillItem -> {
          orderBillItem.setiPadPOSOrderV2Bill(orderBill);
          orderBillItem.setOrderId(order.getOrderId());
        }
      }
    }
  }   

  sessionFactory.getCurrentSession().save(order);
}

Note that this isn't that different from your initial code and thus you should think about whether that conversion would make sense.

Converting your nested loops to a fully sequential stream would be harder and in the end not that different because you can't just flat map orderBill to a stream of orderBillItem. Doing that would not make orderBill available downstream so you'd have to call orderBillItem.setiPadPOSOrderV2Bill(orderBill); before returning the nested stream. That would end up in code very similar to the above and add no benefit because you're not using the returned stream.

Upvotes: 1

kosta.dani
kosta.dani

Reputation: 457

Filter out the nulls ommiting the null checks

     private void createNewOrderV2(IPadPOSOrderDTO iPadPOSOrderDTO) {
          IPadPOSOrderV2 order = mapper.map(iPadPOSOrderDTO, IPadPOSOrderV2.class);
          order.getOrderV2Bills().stream().filter(Objects::nonNull).forEach(e -> createBill(order, e));
          sessionFactory.getCurrentSession().save(order);
       }

       private void createBill(IPadPOSOrderV2 ipadExistingOrderFromDatabase, IPadPOSOrderV2Bill iPadPOSOrderV2Bill) {
          iPadPOSOrderV2Bill.setOrder(ipadExistingOrderFromDatabase);
          iPadPOSOrderV2Bill.getiPadPOSOrderV2BillItems().stream().filter(Objects::nonNull).forEach(e -> {
                e.setiPadPOSOrderV2Bill(iPadPOSOrderV2Bill);
                e.setOrderId(ipadExistingOrderFromDatabase.getOrderId());
          });
       }

By the way your createBill() is called by the createBillItem and also the other way around, is this correct?

Upvotes: 0

Related Questions