Peter Penzov
Peter Penzov

Reputation: 1588

Optimize number for SQL queries

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

Answers (2)

df778899
df778899

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

Andreas
Andreas

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

Related Questions