14

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()));
7
  • Sadly Java doesn't allow for tuple unpacking. Commented May 9, 2016 at 22:11
  • P.S. the question title is terrible - surely the answer is "don't provide incompatible parameters". Commented May 9, 2016 at 22:12
  • 1
    @Boris the Spider: but writing a utility method for converting a BiConsumer<K,V> to a Consumer<Map.Entry<K,V>> is trivial… Commented May 10, 2016 at 8:53
  • 1
    @Boris the Spider: Even worse, Java has no tuples… Commented May 10, 2016 at 8: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. Commented May 10, 2016 at 8:57

2 Answers 2

6

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());
}
Sign up to request clarification or add additional context in comments.

1 Comment

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.
6

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

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

Comments

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.