Optimising multiple streams to single loop

Question

I am trying to find the best way to optimise the converters below to first follow the flow I call convertAndGroupForUpdate, which triggers the conversions and relevant mappings.

Any help to optimise this code would be massively appreciated.

public List<GroupedOrderActionUpdateEntity> convertAndGroupForUpdate(List<SimpleRatifiableAction> actions) {
 List<GroupedOrderActionUpdateEntity> groupedActions = new ArrayList<>();
 Map<String, List<SimpleRatifiableAction>> groupSimple = actions.stream()
 .collect(Collectors.groupingBy(x -> x.getOrderNumber() + x.getActionType()));
 groupSimple.entrySet().stream()
 .map(x -> convertToUpdateGroup(x.getValue()))
 .forEachOrdered(groupedActions::add);
 return groupedActions;
}
public GroupedOrderActionUpdateEntity convertToUpdateGroup(List<SimpleRatifiableAction> actions) {
 List<OrderActionUpdateEntity> actionList = actions.stream().map(x -> convertToUpdateEntity(x)).collect(Collectors.toList());
 return new GroupedOrderActionUpdateEntity(
 actions.get(0).getOrderNumber(),
 OrderActionType.valueOf(actions.get(0).getActionType()),
 actions.get(0).getSource(),
 12345,
 actions.stream().map(SimpleRatifiableAction::getNote)
 .collect(Collectors.joining(", ", "Group Order Note: ", ".")),
 actionList);
}
public OrderActionUpdateEntity convertToUpdateEntity(SimpleRatifiableAction action) {
 return new OrderActionUpdateEntity(action.getId(), OrderActionState.valueOf(action.getState()));
}

• user158037: It should be already fast, without any changes required. some changes might make code cleaner but not faster. Is actionslist fetched from database? Then performance issue might be caused by lazy loading and stackoverflow.com/a/2764474/158037 .

Answer 1

Specifying a downstream Collector for Collectors.groupingBy

There is an alternative Collectors.groupingBy(Function, Collector) method that lets you specify further steps that you want to do with the intermediary List<SimpleRatifiableAction> values after the grouping.

Then, with a bit of renaming, some help from method references, plus some convenience methods like having a SimpleRatifiableAction.getKey():

public String getKey() {
 return getOrderNumber() + getActionType();
}

You can have a method that reads:

// dropping method visibility modifier for brevity
List<GroupedOrderActionUpdateEntity> process(List<SimpleRatifiableAction> actions) {
 return new ArrayList<>(actions.stream()
 .collect(Collectors.groupingBy(SimpleRatifiableAction::getKey,
 Collectors.collectingAndThen(Collectors.toList(), 
 this::createUpdateEntity)))
 .values());
}

Looping once, aggregating multiple values

Inside convertToUpdateGroup(List), now renamed as createUpdateEntity(List), you are streaming twice on the List argument. While this shouldn't be an issue for most cases, there is still an option to just loop once should it be one of the remaining places to optimize (hopefully with some runtime analysis/micro-benchmarking to prove it).

This is achieved by creating the StringJoiner instance yourself (instead of relying on Collectors.joining()). To avoid similar List.get(0) calls, you can also get a reference to it once.

Putting it altogether:

// dropping method visibility modifier for brevity
GroupedOrderActionUpdateEntity createUpdateEntity(List<SimpleRatifiableAction> actions) {
 SimpleRatifiableAction first = actions.get(0);
 StringJoiner joiner = new StringJoiner(", ", "Group Order Note: ", ".");
 List<OrderActionUpdateEntity> updateEntities = new ArrayList<>();
 actions.forEach(v -> {
 joiner.add(v.getNote());
 updateEntities.add(v.createUpdateEntity());
 });
 return new GroupedOrderActionUpdateEntity(
 first.getOrderNumber(),
 OrderActionType.valueOf(first.getActionType()),
 first.getSource(),
 12345,
 joiner.toString(),
 updateEntities);
}

SimpleRatifiableAction.createUpdateEntity() is also another convenience method that you can consider:

public OrderActionUpdateEntity createUpdateEntity() {
 return new OrderActionUpdateEntity(getId(), OrderActionState.valueOf(getState()));
}