I would like to optimize the below code to take an existing ArrayList and make a new list based on the below logic. Does streaming/lambda in Java 8 make it possible?
for (ShipmentTracingDTO tracing : tracings) {
if (tracing.getDestination()) {
newTracings.add(tracing);
break;
}
newTracings.add(tracing);
}
You can do it using IntStream.range and List.subList possibly as :
List<ShipmentTracingDTO> newTracings = tracings.subList(0, IntStream.range(0, tracings.size())
.filter(i -> tracings.get(i).getDestination())
.map(a -> a + 1) // to include the index of occurrence
.findFirst()
.orElse(tracings.size())); // else include all until last element
Note: This is just a view of tracings and hence changing the underlying list tracings would also reflect in changes in newTracings. If you want to decouple these, you can create a new ArrayList<>() from the solutions.
Edit: In comments from Holger, you could alternatively use:
List<ShipmentTracingDTO> newTracings = IntStream.rangeClosed(1, tracings.size())
.filter(i -> tracings.get(i - 1).getDestination()) // i-1 to include this as well
.mapToObj(i -> tracings.subList(0, i)) // Stream<List<ShipmentTracingDTO>>
.findFirst() // Optional<List<ShipmentTracingDTO>>
.orElse(tracings); // or else the entire list
List<ShipmentTracingDTO> newTracings = IntStream.rangeClosed(1, tracings.size()) .filter(i -> tracings.get(i - 1).getDestination()).mapToObj(i -> tracings.subList(0,i)) .findFirst() .orElse(tracings); If a new list is required, just wrap the entire expression in a new ArrayList<>( … )
i-1 part was something that was just not striking my mind while I was thinking to solve this one. Maybe, the same thing to remind me to write the iterative first, optimize it to the best and then approach streams. Thanks again anyway.
If I needed such processing I would divide it into two separate streams:
final int lastElemPosition = tracings.size() - 1;
final int firstElemWithGetDestinationTruePosition = IntStream.range(0, lastElemPosition)
.filter(i -> tracings.get(i).getDestination())
.findFirst()
.orElse(lastElemPosition);
final List<ShipmentTracingDTO> desiredElements = IntStream.rangeClosed(0, firstElemWithGetDestinationTruePosition)
.mapToObj(tracings::get)
.collect(Collectors.toList());
First, I would point out there's not much wrong with the original code that you want to change. Just because Java has streams, you don't have to use them.
To use streams, I would break up the solution into two phases:
OptionalInt indexOfMatchingItem = IntStream.range(0, tracings.size())
.filter(i -> tracings.get(i).getDestination())
.findFirst();
List<ShipmentTracingDTO> newTracings = new ArrayList<>(
indexOfMatchingItem
.map(i -> tracings.subList(0, i + 1))
.orElse(tracings));
The above can be written as a single expression, but splitting it with a suitably named intermediate variable can make the code self-documenting.
I've created a new ArrayList so that it won't be affected by any subsequent changes to the original list tracings. If tracings is immutable, you can skip the construction of a new ArrayList.
The above solution is slightly better performing than the original code in the question, because the ArrayList constructor pre-allocates an array of exactly the required size, and so avoids the overhead of resizing and multiple array copies.
An alternative (perhaps more readable but less performant) solution is:
List<ShipmentTracingDTO> newTracings =
Stream.concat(
tracings.stream().takeWhile(i -> !tracings.get(i).getDestination()),
tracings.stream().dropWhile(i -> !tracings.get(i).getDestination()).limit(1)
).collect(toList());
newTracings = new ArrayList<>(indexOfMatchingItem.isPresent()? tracings.subList(0, 1 + indexOfMatchingItem.get()): tracings). The performance does not only benefit from pre-sizing the array, when the source is an ArrayList, there will be a single Arrays.copyOf operation from the source list’s array.
list.subList(0,list.size()) == list (by identity) in most implementations of subList, so the two variations are equivalent. The isPresent call is often considered a "code smell" and the whole smell of the solution seems to suggest it's a weird requirement - it's not as obvious in the original for-loop version but it becomes more obvious when you try to turn it into functional code.list.subList(0,list.size()) == list does not hold for the commonly used ArrayList, for example. I’m not even sure whether the contract would allow this optimization as subsequently adding an element to the original list invalidates the sublist rather than show through. To those considering isPresent as code smell, new ArrayList<>(indexOfMatchingItem.map(i -> tracings.subList(0, 1 + i)).orElse(tracings)) would do, but lose the advantage of not creating a new object.
takeWhile, but then the part where the element even when the condition is met isadded is trickier since thetakeWhilewould ignore that specific element.for (ShipmentTracingDTO tracing : tracings) { newTracings.add(tracing); if(tracing.getDestination()) break; }