11

I have the following code

public static List<Integer> topKFrequent(int[] nums, int k) {
  List<Integer> myList = new ArrayList<>();
  HashMap<Integer, Integer> map = new HashMap<>();

  for (int n : nums) {
    if (!map.containsKey(n)) map.put(n, 1);
    else map.put(n, map.get(n) + 1);
  }

  map.entrySet().stream()
    .sorted(Map.Entry.<Integer, Integer>comparingByValue().reversed())
    .limit(k)
    .forEach((key, value) -> myList.add(key));

  return myList;
}

The forEach throws the error

Error:(20, 16) java: incompatible types: incompatible parameter types in lambda expression

How can I fix/avoid this error? I'm not quite sure how to apply the answer here that explains the problem: Lambda Expression and generic method

Edit:

Given the answer, the correction is to replace the lambda inside the forEach with

.forEach((entry) -> myList.add(entry.getKey()));
Community
  • 1
  • 1
m0meni
  • 16,006
  • 16
  • 82
  • 141
  • Sadly Java doesn't allow for tuple unpacking. – Boris the Spider May 09 '16 at 22:11
  • P.S. the question title is terrible - surely the answer is "don't provide incompatible parameters". – Boris the Spider May 09 '16 at 22:12
  • @BoristheSpider sorry this is my first time using lambda's and streams and I wasn't quite sure what the problem was after reading that answer about generics and functional interfaces http://stackoverflow.com/questions/22588518/lambda-expression-and-generic-method. – m0meni May 09 '16 at 22:13
  • 1
    @Boris the Spider: but writing a utility method for converting a `BiConsumer` to a `Consumer>` is trivial… – Holger May 10 '16 at 08:53
  • @Holger sure, and the converse is even easier. Doesn't mean Java unpacks tuples... – Boris the Spider May 10 '16 at 08:54
  • 1
    @Boris the Spider: Even worse, Java has no tuples… – Holger May 10 '16 at 08:55
  • 1
    @AR7: note that for your first loop, you can use the stream solution of the accepted answer, but when you use a loop, you can replace `if (!map.containsKey(n)) map.put(n, 1); else map.put(n, map.get(n) + 1);` with `map.merge(n, 1, Integer::sum);`. So there’s not only the Stream API, the Collection API also has lots of useful new methods. – Holger May 10 '16 at 08:57

2 Answers2

6

entrySet() returns a set of Pair<K, V>.

forEach()'s lambda therefore takes a single parameter of that type; not two integer parameters.

SLaks
  • 868,454
  • 176
  • 1,908
  • 1,964
4

You are going about it in a java7-ish way. Modifying external data structures from inside forEach is not how Streams API was meant to be used. Streams API documentation specifically warns against such use in the Side-Effects section of java.util.stream package summary

Instead of appending to list or map from inside forEach, use collect:

import static java.util.Comparator.reverseOrder;
import static java.util.Map.Entry.comparingByValue;
import static java.util.stream.Collectors.counting;
import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.toList;


public static List<Integer> topKFrequent(int[] nums, int k) {
    Map<Integer, Long> freq = Arrays.stream(nums).boxed()
            .collect(groupingBy(x->x, counting()));

    return freq.entrySet()
            .stream()
            .sorted(comparingByValue(reverseOrder()))
            .limit(k)
            .map(Map.Entry::getKey)
            .collect(toList());
}
Misha
  • 27,433
  • 6
  • 62
  • 78
  • 1
    Ah ok that's very cool. I haven't written Java in about 2 years, which is why it's currently a mix. It makes sense to not have side effects in streams, but I wasn't quite sure what I was doing yet. – m0meni May 09 '16 at 22:30