Reputation: 1588
I use this code to to generate chart with data:
@GetMapping("/terminals")
public ResponseEntity<Map<String, List<TopTerminalsDTO>>> getTopTerminals(
@RequestParam(value = "start_date", required = true) String start_date,
@RequestParam(value = "end_date", required = true) String end_date) {
final List<PaymentTransactionsDailyFacts> list = dashboardService.findTop_Terminals(start_dateTime, end_dateTime);
final Collector<PaymentTransactionsDailyFacts, List<TopTerminalsDTO>, List<TopTerminalsDTO>> terminalsCollector = Collector
.of(ArrayList::new, (terminals, p) -> terminals.add(mapper.toTopTerminalsDTO(p)),
(accumulator, terminals) -> {
accumulator.addAll(terminals);
return accumulator;
});
final Map<String, List<TopTerminalsDTO>> final_map = list.stream().filter(p -> p.getTerminal_id() != null)
.collect(Collectors.groupingBy(p -> getTerminalName(p.getTerminal_id()), terminalsCollector));
return ResponseEntity.ok(final_map);
}
private String getTerminalName(Integer id) {
Optional<Terminals> obj = terminalService.findById(id);
return obj.map(Terminals::getName).orElse("");
}
But I noticed that getTerminalName
is called more than 10 times to translate the name from number. Do you know how I can reduce the number of calls with some optimization?
Upvotes: 1
Views: 213
Reputation: 10931
Sounds like a case for a temporary cache, scoped to just this request, or perhaps longer if the terminal names are stable enough.
Clearly something like ehCache behind the scenes would suit this well, but I am often tempted with a tactical bit of caching, especially if I don't want to keep the cached values beyond this immediate request.
For example:
TerminalNameCache cache = new TerminalNameCache();
final Map<String, List<TopTerminalsDTO>> final_map = list.stream()
.filter(p -> p.getTerminal_id() != null)
.collect(Collectors.groupingBy(
p -> cache.getTerminalName(p.getTerminal_id()),
terminalsCollector));
Then the TerminalNameCache
is just an inner class in the parent Controller class. This is to allow it to call the existing private String getTerminalName(Integer id)
method from the question (shown as on ParentControllerClass
below):
private class TerminalNameCache {
private final Map<Integer, String> cache = new ConcurrentHashMap<>();
private String getTerminalName(Integer id) {
return cache.computeIfAbsent(id,
id2 -> ParentControllerClass.this.getTerminalName(id2));
}
}
If this looks like emerging into a pattern, it would be worth refactoring the caching to something more re-useable. E.g. this could based around caching calls to a generic function:
public class CachedFunction<T, R> implements Function<T, R> {
private final Function<T, R> function;
private final Map<T, R> cache = new ConcurrentHashMap<>();
public CachedFunction(Function<T, R> function) {
this.function = function;
}
@Override
public R apply(T t) {
return cache.computeIfAbsent(t, t2 -> function.apply(t2));
}
}
This would then be used like this:
CachedFunction<Integer, String> cachedFunction = new CachedFunction<>(
id -> getTerminalName(id));
final Map<String, List<TopTerminalsDTO>> final_map = list.stream()
.filter(p -> p.getTerminal_id() != null)
.collect(Collectors.groupingBy(
p -> cachedFunction.apply(p.getTerminal_id()),
terminalsCollector));
Upvotes: 1
Reputation: 159096
Modify findTop_Terminals
and PaymentTransactionsDailyFacts
to include the name (using a SQL LEFT JOIN
clause).
Alternative, scan the list for all terminal ids, then call a List<Terminals> list = terminalService.findByIds(idList);
method to get all those terminals using a SQL IN
clause.
Note: Beware limit on how many ?
markers can be in a SQL statement.
Then build a Map<Integer, String>
mapping terminal id to name, and replace getTerminalName
method with map lookup.
Upvotes: 2