Reputation: 43
I am trying to find the best way to optimise the converters below to follow the flow I call 'convertAndGroupForUpdate' first 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()));
}
Upvotes: 3
Views: 289
Reputation: 298599
You can’t elide a grouping operation, but you don’t need to store the intermediate result in a local variable.
Further, you should not add to a list manually, when you can collect
to a List
. Just do it like you did in the other method.
Also, creating a grouping key via string concatenation is tempting, but very dangerous, depending on the contents of the properties, the resulting strings may clash. And string concatenation is rather expensive. Just create a list of the property values, as long as you don’t modify it, it provides the right equality semantics and hash code implementation.
If you want to process the values of a map only, don’t call entrySet()
, to map each entry via getValue()
. Just use values()
in the first place.
public List<GroupedOrderActionUpdateEntity> convertAndGroupForUpdate(
List<SimpleRatifiableAction> actions) {
return actions.stream()
.collect(Collectors.groupingBy( // use List.of(…, …) in Java 9 or newer
x -> Arrays.asList(x.getOrderNumber(), x.getActionType())))
.values().stream()
.map(x -> convertToUpdateGroup(x))
.collect(Collectors.toList());
}
Since convertToUpdateGroup
is processing the list of actions of each group multiple times, there is not much that can be simplified and I wouldn’t inline it either. If there was only one operation, e.g. joining them to a string, you could do that right in the groupingBy
operation, but there is no simply way to collect to multiple results.
Upvotes: 4