sum and max values in a single iteration

user_1106134 0
Viewed 244

I have a List of a custom CallRecord objects

public class CallRecord {

    private String callId;
    private String aNum;
    private String bNum;
    private int seqNum;
    private byte causeForOutput;
    private int duration;

    private RecordType recordType;
.
.
.
}

There are two logical conditions and the output of each is:

  1. Highest seqNum, sum(duration)
  2. Highest seqNum, sum(duration), highest causeForOutput

As per my understanding, Stream.max(), Collectors.summarizingInt() and so on will either require several iterations for the above result. I also came across a thread suggesting custom collector but I am unsure.

Below is the simple, pre-Java 8 code that is serving the purpose:

if (...) {

    for (CallRecord currentRecord : completeCallRecords) {
        highestSeqNum = currentRecord.getSeqNum() > highestSeqNum ? currentRecord.getSeqNum() : highestSeqNum;
        sumOfDuration += currentRecord.getDuration();
    }

} else {
    byte highestCauseForOutput = 0;

    for (CallRecord currentRecord : completeCallRecords) {
        highestSeqNum = currentRecord.getSeqNum() > highestSeqNum ? currentRecord.getSeqNum() : highestSeqNum;
        sumOfDuration += currentRecord.getDuration();

        highestCauseForOutput = currentRecord.getCauseForOutput() > highestCauseForOutput ? currentRecord.getCauseForOutput() : highestCauseForOutput;
        }

}
collectors java-stream java-8
3 Answers

Your desire to do everything in a single iteration is irrational. You should strive for simplicity first, performance if necessary, but insisting on a single iteration is neither.

The performance depends on too many factors to make a prediction in advance. The process of iterating (over a plain collection) itself is not necessarily an expensive operation and may even benefit from a simpler loop body in a way that makes multiple traversals with a straight-forward operation more efficient than a single traversal trying to do everything at once. The only way to find out, is to measure using the actual operations.

Converting the operation to Stream operations may simplify the code, if you use it straight-forwardly, i.e.

int highestSeqNum=
  completeCallRecords.stream().mapToInt(CallRecord::getSeqNum).max().orElse(-1);
int sumOfDuration=
  completeCallRecords.stream().mapToInt(CallRecord::getDuration).sum();
if(!condition) {
  byte highestCauseForOutput = (byte)
    completeCallRecords.stream().mapToInt(CallRecord::getCauseForOutput).max().orElse(0);
}

If you still feel uncomfortable with the fact that there are multiple iterations, you could try to write a custom collector performing all operations at once, but the result will not be better than your loop, neither in terms of readability nor efficiency.

Still, I’d prefer avoiding code duplication over trying to do everything in one loop, i.e.

for(CallRecord currentRecord : completeCallRecords) {
    int nextSeqNum = currentRecord.getSeqNum();
    highestSeqNum = nextSeqNum > highestSeqNum ? nextSeqNum : highestSeqNum;
    sumOfDuration += currentRecord.getDuration();
}
if(!condition) {
    byte highestCauseForOutput = 0;
    for(CallRecord currentRecord : completeCallRecords) {
        byte next = currentRecord.getCauseForOutput();
        highestCauseForOutput = next > highestCauseForOutput? next: highestCauseForOutput;
    }
}
user_27114880

With Java-8 you can resolved it with a Collector with no redudant iteration.

Normally, we can use the factory methods from Collectors, but in your case you need to implement a custom Collector, that reduces a Stream<CallRecord> to an instance of SummarizingCallRecord which cotains the attributes you require.

Mutable accumulation/result type:

class SummarizingCallRecord {
  private int highestSeqNum = 0;
  private int sumDuration = 0;

  // getters/setters ...      
}

Custom collector:

BiConsumer<SummarizingCallRecord, CallRecord> myAccumulator = (a, callRecord) -> {
  a.setHighestSeqNum(Math.max(a.getHighestSeqNum(), callRecord.getSeqNum()));
  a.setSumDuration(a.getSumDuration() + callRecord.getDuration());
};

BinaryOperator<SummarizingCallRecord> myCombiner = (a1, a2) -> {
  a1.setHighestSeqNum(Math.max(a1.getHighestSeqNum(), a2.getHighestSeqNum()));
  a1.setSumDuration(a1.getSumDuration() + a2.getSumDuration());
  return a1;
};

Collector<CallRecord, SummarizingCallRecord, SummarizingCallRecord> myCollector = 
  Collector.of(
    () -> new SummarizinCallRecord(), 
    myAccumulator, 
    myCombiner,
    // Collector.Characteristics.CONCURRENT/IDENTITY_FINISH/UNORDERED
  );

Execution example:

List<CallRecord> callRecords = new ArrayList<>();
callRecords.add(new CallRecord(1, 100));
callRecords.add(new CallRecord(5, 50));
callRecords.add(new CallRecord(3, 1000));


SummarizingCallRecord summarizingCallRecord  = callRecords.stream()
  .collect(myCollector);

// Result:
//    summarizingCallRecord.highestSeqNum = 5
//    summarizingCallRecord.sumDuration = 1150
user_27868320

You don't need and should not implement the logic by Stream API because the tradition for-loop is simple enough and the Java 8 Stream API can't make it simpler:

int highestSeqNum = 0;
long sumOfDuration = 0;
byte highestCauseForOutput = 0; // just get it even if it may not be used. there is no performance hurt.
for(CallRecord currentRecord : completeCallRecords) {
    highestSeqNum = Math.max(highestSeqNum, currentRecord.getSeqNum());
    sumOfDuration += currentRecord.getDuration();
    highestCauseForOutput = Math.max(highestCauseForOutput, currentRecord.getCauseForOutput());
}

// Do something with or without highestCauseForOutput.
user_83141750
Related