Setting value of item in a List given next item with a certain value

user_1501171 0
Viewed 83

I have a list of SalesOrders, both Inactive and Active. I want to loop through this entire list and set the length of time each SalesOrder was Active for. (The list is already ordered by StartTime)

My loop is as follows:

for(int i = 0; i < salesOrders.size(); i++) {
    if(salesOrders.get(i).getStatus() == RecordStatus.ACTIVE) {
        ELDDutyLog activeSalesOrder = salesOrders.get(i);

        for(int j = i + 1; j < salesOrders.size(); j++) {
            if(salesOrders.get(j).getStatus() == RecordStatus.ACTIVE) {
                ELDDutyLog nextActiveSalesOrder = salesOrders.get(j);

                salesOrders.get(i).setActiveDurationMinutes(
                    Minutes.minutesBetween(
                        activeSalesOrder.getStartTime(),
                        nextActiveSalesOrder.getStartTime()
                    ).getMinutes()
                );
                break;
            }
        }
    }
}
return salesOrders;

While this approach works, I feel there must be a better way of doing this, possibly by using Java 8? Can anyone provide me with any direction around this?

java-8 java
3 Answers

You do not have to loop the entire list again, each time skipping the inactive sales orders. Instead, you can filter the active sales orders once, then you always know that the next active sales order is the next order in the filtered list.

Something like this (not tested):

List<ELDDutyLog> activeSalesOrders = salesOrders.stream()
        .filter(so -> so.getStatus() == RecordStatus.ACTIVE)
        .collect(Collectors.toList());

for (int i = 0; i < activeSalesOrders.size() - 1; i++) {
    activeSalesOrders.get(i).setActiveDurationMinutes(
                    Minutes.minutesBetween(
                        activeSalesOrders.get(i).getStartTime(),
                        activeSalesOrders.get(i+1).getStartTime()
                    ).getMinutes()
                );
}
user_16396250

I hope I understand correctly. Try (not tested):

salesOrders.stream()
  .filter(salesOrder -> salesOrder.getStatus() == RecordStatus.ACTIVE)
  .reduce((current, next) -> {
    current.setActiveDurationMinutes(
      Minutes.minutesBetween(
        current.getStartTime(),
        next.getStartTime()
      ).getMinutes()
    );
    return next;
  });

To be honest... it's a bit of a weird use for reduce, but I think it'll work.

user_69025430

There is no need to nest two loops; the nextActiveSalesOrder you have searched for to calculate the argument for the setActiveDurationMinutes call, is the same object that will become the activeSalesOrder you are searching for in the next iterations of the outer loop. Besides that, you are unnecessarily calling salesOrders.get(i) in the inner operation despite you already have that value stored in activeSalesOrder.

ELDDutyLog activeSalesOrder = null;
for(ELDDutyLog nextActiveSalesOrder: salesOrders) {
    if(nextActiveSalesOrder.getStatus() != RecordStatus.ACTIVE) continue;
    if(activeSalesOrder != null)
        activeSalesOrder.setActiveDurationMinutes(
            Minutes.minutesBetween(
                activeSalesOrder.getStartTime(),
                nextActiveSalesOrder.getStartTime()
            ).getMinutes()
        );
    activeSalesOrder = nextActiveSalesOrder;
}

Besides that… what kind of API requires you to write something like Minutes.minutesBetween(a, b).getMinutes()? How many times can an API need to be told that you want minutes?
Compare to java.time:

  1. ChronoUnit.MINUTES.between(a, b) (returns a long already) or
  2. a.until(b, ChronoUnit.MINUTES) (dito, still no redundancy)
user_27114880
Related