Is there way of optimizing below code further using java8?

Viewed 464

I encountered below code in my project. I was wondering if it can be optimized further may be by using java 8 streams or by collection APIs in general.

private Set<Student> getFilteredSet() {
    Set<Student> unfilteredSet = getAllStudents();
    Set<Student> adminAreaSet = getAdminStudents();

    Set<String> adminAreaID = new HashSet<>();
    Set<Student> filteredSet = new HashSet<>();

    for (final Student student : adminAreaSet) {
        adminAreaID.add(student.getId());
    }
    for (final Student student : unfilteredSet) {
        if (adminAreaID.contains(student.getId())) {
            filteredSet.add(student);
        }
    }   
    return filteredSet;
}

Note: unfilteredSet and adminAreaSet hold different child types of Student

2 Answers

Since the question is tagged with , one way to improve readability of the code could be to transform it as :

Set<String> adminAreaID = getAdminStudents().stream()
        .map(Student::getId)
        .collect(Collectors.toSet());

return getAllStudents().stream()
        .filter(student -> adminAreaID.contains(student.getId()))
        .collect(Collectors.toSet());

According to your comment, you're looking for speed optimization. There is a lot of post comparing Stream and Collection and even more on whole Internet. I recommend you by example to have a look at this question which compares speed performance between Streams and old for each loop: Java 8: performance of Streams vs Collections. As using Stream creates a lot of intermediate objects and call intermediate methods, it seems normal to be slower than basic for each loop. However you can use stream to have more readable/smaller code.

To answer the question, I think your code is already very good considering speed performance. All I see is that you should initialize adminAreaID because you know exactly the size it will have :

    Set<String> adminAreaID = new HashSet<>(adminAreaSet.size(), 1.);

By setting size and load factor, you ensure no time will be used to grow up your set. According to https://docs.oracle.com/javase/7/docs/api/java/util/HashMap.html :

The load factor is a measure of how full the hash table is allowed to get before its capacity is automatically increased.

You have to set it at 1. because you won't get higher the adminAreaSet size. Moreover if you let it at .75 (the default value), your Set will grow up once when the loop will reach 75% of its capacity which is useless.

If you have no memory concern, you shoul do the same with filteredSet :

    Set<Student> filteredSet = new HashSet<>(unfilteredSet.size(), 1.);

In fact, as you filter unfilteredSet, you won't reach the max capacity but it will ensure you that filteredSet will not grow up during its filling.

Related