2
\$\begingroup\$

I have two maps map1 and map2 . I want to merge both maps and then sort in desc order and get top 5. In case of duplicate keys in merge I need to sum the values. I have the following code that works:

Map<String, Long> topFive = (Stream.concat(map1.entrySet().stream(), map2.entrySet().stream())
        .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, Long::sum)))
        .entrySet().stream().sorted(Map.Entry.comparingByValue(Comparator.reverseOrder()))
        .limit(5).collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue,
            (v1,v2) -> v1,
            LinkedHashMap::new));

But I would like to know if there is a better solution.

\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

The question for a "better" solution is bound to be quite opinion-based. As far as the actual code goes, I think you are quite close to the optimum. As for readability... well, Roland already shared his initial thoughts and I totally agree. This is bound to be a nightmare for the poor maintenance programmer.

Thus: a suggestion which is nothing new, but just a little rearangement:

private Map<String, Long> getHistogramTopFive(Map<String, Long> map1, Map<String, Long> map2) {
    Map<String, Long> mergedMap = (Stream.concat(map1.entrySet().stream(), map2.entrySet().stream())
        .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, Long::sum)));

    Map<String, Long> topFive = mergedMap.entrySet().stream()
        .sorted(Map.Entry.comparingByValue(Comparator.reverseOrder()))
        .limit(5)
        .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue,
                (v1,v2) -> v1,
                LinkedHashMap::new));

    return topFive;
}

By putting it into a method and naming this method, you document your business intention. Furthermore, by breaking up the long command chain with a new mergedMap variable in between, the code gains much in readability.

The additional line-breaks in the second part are what I consider best practices in streaming API: each new operation on a new line.

\$\endgroup\$
0
\$\begingroup\$

When I first saw the code it looked frightening because it is a solid rectangle of characters, with no apparent structure. It looks like you wrote a novel in a single paragraph.

Then I took a deep breath and slowly read the code from the beginning to the end, which made me think: yes, it's exactly what you deccibed in natural language above. It's sad that the code is so much longer than your description.

Basically you are dealing with a histogram, which is a Map<T, Long> sorted by the values. Therefore, try to find a third-party library that offers a histogram class, and then your code may read:

var topFive = Histogram.create(map1).addAll(map2).top(5);

I don't know of such a library from the top of my head, but it surely exists.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.