1

I am trying to write my own CollectionUtils helper class that other application will use. Here is my first method

    
    public static <T, K, V> 
    Map<K, List<V>> listToMap(List<T> inputList, 
            Function<? super T, ? extends K> keyMapper, 
            Function<? super T, ? extends V> valueMapper)
    {
        Collector c = Collectors.toMap(keyMapper, valueMapper);
        return inputList.stream()
                .collect(c);
        
        
    }
    
    public void test()
    {
        // trying to get a Map<String, List<String>> showing student's name and their activities.
        listToMap(StudentDatabase.getAllStudents(), Student::getName, Student::getActivites);
    }

However, I am getting lots of compilation error that I do not understand how to solve. Can I get some help here? Is there any third party library that already does that (but it has to be using java stream api) I can use so that I do not need to write my own?

I tried above and having compilation issue.

Improve this question
5
  • would you mind provide me the example? I am trying this but still compilation issue:public static <R, A, T, K, V> R listToMap(List<T> inputList, Function<? super T, ? extends K> keyMapper, Function<? super T, ? extends V> valueMapper) { Collector c = Collectors.toMap(keyMapper, valueMapper); return inputList.stream() .collect(c); } Commented Feb 13, 2023 at 0:59
  • Why are you using a raw Collector? Commented Feb 13, 2023 at 1:03
  • @bluemountain There was no reason to add R or A to the type parameters. You already have all the necessary generic information. Part of the problem is that c is declared to be a raw Collector. You need to parameterize it. My answer goes into more detail. Commented Feb 13, 2023 at 1:16
  • This whole helper seems pretty pointless. Commented Feb 13, 2023 at 1:22
  • @shmosel Eh, maybe. If this pattern is used many times throughout the code (or other projects), then I can see making this helper method. Commented Feb 13, 2023 at 1:35

1 Answer 1

Reset to default
2

There's a couple of problems with the current code:

  1. The Collector interface is generic. You should parameterize it like you're doing for all the other generic types in the code. See What is a raw type and why shouldn't we use it? for more information.

  2. You've defined the return type as Map<K, List<V>>. That would seem to indicate you're trying to implement a grouping operation. However, there are three other parts of your code indicating otherwise:

    • You use Collectors#toMap(Function,Function)

    • Your valueMapper maps to V, not List<V>

    • You call listToMap with Student::getActivities as an argument for the valueMapper, and I can only assume that method returns a list of activities (or some other collection).

    So, given all that, you should change the return type to Map<K, V>. That gives the caller full control over the value type of the map, rather than forcing them to use a list. But if you are trying to implement a grouping operation, and you always want the value type to be a List<V>, then consider using Collectors#groupingBy(Function,Collector) instead.

Fixing those two things will give you something like:

public static <T, K, V> Map<K, V> listToMap(
        List<T> list,
        Function<? super T, ? extends K> keyMapper,
        Function<? super T, ? extends V> valueMapper) {
    Collector<T, ?, Map<K, V>> collector = Collectors.toMap(keyMapper, valueMapper);
    return list.stream().collect(collector);
}

And here's a minimal example using the above:

import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Collector;
import java.util.stream.Collectors;

public class Main {

    public record Student(String name, List<String> activities) {}

    public static <T, K, V> Map<K, V> listToMap(
            List<T> list, 
            Function<? super T, ? extends K> keyMapper, 
            Function<? super T, ? extends V> valueMapper) {
        Collector<T, ?, Map<K, V>> collector = Collectors.toMap(keyMapper, valueMapper);
        return list.stream().collect(collector);
    }

    public static void main(String[] args) {
        List<Student> students = List.of(
            new Student("John", List.of("Piano", "Running")),
            new Student("Jane", List.of("Soccer", "Video Games")),
            new Student("Bob", List.of("Snowboarding"))
        );
        Map<String, List<String>> map = listToMap(students, Student::name, Student::activities);
        System.out.println(map);
    }
}

Output:

{Bob=[Snowboarding], John=[Piano, Running], Jane=[Soccer, Video Games]}
Improve this answer
Sign up to request clarification or add additional context in comments.

9 Comments

I think we are getting close. Now we have the listToMap method fixed. However, I am getting issue on this line when I am trying to invoke that method, it is complaining here Student::getName, Student::getActivites saying "The type Student does not define getName(T) that is applicable here" and "The type Student does not define getActivites(T) that is applicable here"
@bluemountain I added a minimal example using the listToMap method. Note I made Student a record, so the "getters" are named e.g., name() instead of getName(), but that's mostly immaterial. You can see it gives what you want. If you're still having issues, then please describe the error you're getting.
@Eritrean That's true. Though the fix for that is to use the toMap overload that accepts a BinaryOperator (the "merge function"). Unless there is a way the OP wants to merge duplicate keys in all cases, that means adding a BinaryOperator argument to the listToMap method (possibly via an overload). And at that point, I'm of the opinion that creating this helper method loses any benefits it may have been providing. It'd be better to just stream-and-collect "directly", given the Collectors class is already essentially a utility class (i.e., nothing but helper methods).
I don’t think that this utility class had a benefit in the first place. Being able to write listToMap(StudentDatabase.getAllStudents(), Student::getName, Student::getActivites) instead of StudentDatabase.getAllStudents().stream() .collect(toMap(Student::getName, Student::getActivites)) does not look like an improvement at all.
The only additional information, the listToMap name provides, is that getAllStudents() returns a List, as the “toMap” is contained in the stream operation as well. But that’s the information I don’t care about, as the whole operation would also work if getAllStudents() returned a different Collection type, like a Set, or anything else that has an appropriate stream() method. Granted, the stream operation has slightly more syntactic elements, on the other hand, those elements are easier to read if you’re used to left-to-right writing systems, in contrast to the nested invocation
|

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.