0

I am new to java8 stream & sorry about the stupid question . Here is my code which i am trying to create a map of id & value, but i am getting this error, not able to fix. Can anyone help me what is the alternative?

    public static Map<Integer, String> findIdMaxValue(){
        Map<Integer, Map<String, Integer>> attrIdAttrValueCountMap = new HashMap<>();
        Map<Integer, String> attrIdMaxValueMap = new HashMap<>(); 
                attrIdAttrValueCountMap.forEach((attrId, attrValueCountMap) -> {
                    attrValueCountMap.entrySet().stream().sorted(this::compareAttrValueCountEntry).findFirst().ifPresent(e -> {
                        attrIdMaxValueMap.put(attrId, e.getKey());
                    });
                });
    }

and sorting method

public static int compareAttrValueCountEntry(Map.Entry<String, Integer> e1, Map.Entry<String, Integer> e2) {
    int diff = e1.getValue() - e2.getValue();
    if (diff != 0) {
        return -diff;
    }
    return e1.getKey().compareTo(e2.getKey());
}

I am getting this error

"Cannot use this in a static context"
Stefan Zobel
  • 3,182
  • 7
  • 28
  • 38
Sthita
  • 1,750
  • 2
  • 19
  • 38
  • 2
    Try to use the actual class name instead of `this`. – VHS Mar 02 '17 at 01:38
  • I see, so what is the solution ? I am trying to iterate the map & trying to sort it attrValueCountMap.entrySet().stream().sorted(this::compareAttrValueCountEntry) – Sthita Mar 02 '17 at 01:44

2 Answers2

2

Since the method compareAttrValueCountEntry is declared static,

replace the method reference

this::compareAttrValueCountEntry

with

<Yourclass>::compareAttrValueCountEntry

VHS
  • 9,534
  • 3
  • 19
  • 43
  • I found a better approach http://stackoverflow.com/questions/29567575/sort-map-by-value-using-java-8 – Sthita Mar 02 '17 at 01:53
2

There are several issues with your code. While this::compareAttrValueCountEntry would be easy to fix by changing it to ContainingClassName::compareAttrValueCountEntry, this method is unnecessary as there are several factory methods like Map.Entry.comparingByKey, Map.Entry.comparingByValue, Comparator.reversed and Comparator.thenComparing, which can be combined to achieve the same goal

This guards you from the errors made within compareAttrValueCountEntry. It’s tempting to compare int values by subtracting, but this is error prone as the difference between two int values doesn’t always fit into the int range, so overflows can occur. Also, negating the result for reversing the order is broken, as the value might be Integer.MIN_VALUE, which has no positive counterpart, hence, negating it will overflow back to Integer.MIN_VALUE instead of changing the sign.

Instead of looping via forEach to add to another map, you may use a cleaner stream operation producing the map and you can simplify sorted(…).findFirst() to min(…) which in not only shorter, but a potentially cheaper operation.

Putting it together, we get

Map<Integer, String> attrIdMaxValueMap =
    attrIdAttrValueCountMap.entrySet().stream()
        .filter(e -> !e.getValue().isEmpty())
        .collect(Collectors.toMap(Map.Entry::getKey,
            e -> e.getValue().entrySet().stream()
                .min(Map.Entry.<String, Integer>comparingByValue().reversed()
                        .thenComparing(Map.Entry.comparingByKey())).get().getKey()));

Note that I prepended a filter operation rejecting empty maps, which ensures that there will always be a matching element, so there is no need to deal with ifPresent or such alike. Instead, Optional.get can be called unconditionally.

Since this method is called findIdMaxValue, there might be a desire to reflect that by calling max on the Stream instead of min, wich is only a matter of which comparator to reverse:

Map<Integer, String> attrIdMaxValueMap =
    attrIdAttrValueCountMap.entrySet().stream()
        .filter(e -> !e.getValue().isEmpty())
        .collect(Collectors.toMap(Map.Entry::getKey,
            e -> e.getValue().entrySet().stream()
                .max(Map.Entry.<String, Integer>comparingByValue()
                       .thenComparing(Map.Entry.comparingByKey(Comparator.reverseOrder())))
                .get().getKey()));

Unfortunately, such constructs hit the limitations of the type inference, which requires us to either, use nested constructs (like Map.Entry.comparingByKey(Comparator.reverseOrder()) instead of Map.Entry.comparingByKey().reversed()) or to insert explicit types, like with Map.Entry.<String, Integer>comparingByValue(). In the second variant, reversing the second comparator, we are hitting the litimation twice…

In this specific case, there might be a point in creating the comparator only once, keeping it in a variable and reuse it within the stream operation:

Comparator<Map.Entry<String, Integer>> valueOrMinKey
    = Map.Entry.<String, Integer>comparingByValue()
         .thenComparing(Map.Entry.comparingByKey(Comparator.reverseOrder()));

Map<Integer, String> attrIdMaxValueMap =
    attrIdAttrValueCountMap.entrySet().stream()
        .filter(e -> !e.getValue().isEmpty())
        .collect(Collectors.toMap(Map.Entry::getKey,
            e -> e.getValue().entrySet().stream().max(valueOrMinKey).get().getKey()));
Holger
  • 285,553
  • 42
  • 434
  • 765