CODEWITHSUNDEEP
javajava-streamgroupingcounting
Samuel Philipp
Samuel Philipp

Reputation: 11042

Java stream collect counting to field

Is it possible use Collectors.groupingBy() with Collectors.counting() to count to the field of a custom object instead of creating a map and transforming it afterwards.

I have a list of users, like this:

public class User {
    private String firstName;
    private String lastName;
    // some more attributes

    // getters and setters
}

I want to count all users with the same first and last name. Therefore I have custom object looking like this:

public static class NameGroup {
    private String firstName;
    private String lastName;
    private long count;

    // getters and setters
}

I can collect the name groups using this:

List<NameGroup> names = users.stream()
        .collect(Collectors.groupingBy(p -> Arrays.asList(p.getFirstName(), p.getLastName()), Collectors.counting()))
        .entrySet().stream()
        .map(e -> new NameGroup(e.getKey().get(0), e.getKey().get(1), e.getValue()))
        .collect(Collectors.toList());

With this solution I have to group the users first to a map and transform it afterwards to my custom object. Is it possible to count all names directly to nameGroup.count to avoid iterating twice over the list (or map) and improve the performance?

Upvotes: 5

Views: 5539

Answers (5)

WJS
WJS

Reputation: 40034

I don't know what your requirements are but I modified your NameGroup class to accept a User class instead of first and last names. This then negated the need for for selecting the values from the intermediate stream of List and just from a stream of User. But it still requires the map.

      List<NameGroup> names =
            users.stream().collect(Collectors.groupingBy(p -> p,Collectors.counting()))
                          .entrySet().stream()
                          .map(e -> new NameGroup(e.getKey(), e.getValue())).collect(
                              Collectors.toList());

Upvotes: 0

w4bo
w4bo

Reputation: 905

public static class NameGroup {
    // ...
    @Override
    public boolean equals(Object other) {
        final NameGroup o = (NameGroup) other;
        return firstName.equals(o.firstName) && lastName.equals(o.lastName);
    }
    @Override
    public int hashCode() {
        return Objects.hash(firstName, lastName);
    }
    @Override
    public String toString() {
        return firstName + " " + lastName + " " + count;
    }
}

public static void main(String[] args) throws IOException {
    List<User> users = new ArrayList<>();
    users.add(new User("fooz", "bar"));
    users.add(new User("fooz", "bar"));
    users.add(new User("foo", "bar"));
    users.add(new User("foo", "bar"));
    users.add(new User("foo", "barz"));
    users.stream()
        .map(u -> new NameGroup(u.getFirstName(), u.getLastName(), 1L))
        .reduce(new HashMap<NameGroup, NameGroup>(), (HashMap<NameGroup, NameGroup> acc, NameGroup e) -> {
            acc.compute(e, (k, v) -> v == null ? e : new NameGroup(e.firstName, e.lastName, e.count + acc.get(e).count));
            return acc;
        }, (a, b) -> {
            b.keySet().forEach(e -> a.compute(e, (k, v) -> v == null ? e : new NameGroup(e.firstName, e.lastName, e.count + a.get(e).count)));
            return a;
        }).values().forEach(x -> System.out.println(x));
}

This will output

fooz bar 2
foo barz 1
foo bar 2

Upvotes: 0

Andreas
Andreas

Reputation: 159086

You can minimize allocations of intermediate objects, e.g. all the Arrays.asList(...) objects, by build a map yourself, instead of using streaming.

This relies on the fact that your NameGroup is mutable.

To even make the code simpler, lets add two helpers to NameGroup:

public static class NameGroup {
    // fields here

    public NameGroup(User user) {
        this.firstName = user.getFirstName();
        this.lastName = user.getLastName();
    }

    public void incrementCount() {
        this.count++;
    }

    // other constructors, getters and setters here
}

With that in place, you can implement the logic like this:

Map<User, NameGroup> map = new TreeMap<>(Comparator.comparing(User::getFirstName)
                                                   .thenComparing(User::getLastName));
users.stream().forEach(user -> map.computeIfAbsent(user, NameGroup::new).incrementCount());
List<NameGroup> names = new ArrayList<>(map.values());

Or if you don't actually need a list, the last line can be simplified to:

Collection<NameGroup> names = map.values();

Upvotes: 1

Naman
Naman

Reputation: 31878

Not very clean but you can possibly do it as :

List<NameGroup> convertUsersToNameGroups(List<User> users) {
    return new ArrayList<>(users.stream()
            .collect(Collectors.toMap(p -> Arrays.asList(p.getFirstName(), p.getLastName()),
                    p -> new NameGroup(p.getFirstName(), p.getLastName(), 1L),
                    (nameGroup1, nameGroup2) -> new NameGroup(nameGroup1.getFirstName(), nameGroup1.getLastName(),
                            nameGroup1.getCount() + nameGroup2.getCount()))).values());
}

Upvotes: 2

Louis Wasserman
Louis Wasserman

Reputation: 198033

You could collect directly to NameGroup.count, but it would be less efficient than what you have, not more.

Internally, the map is being used to maintain a data structure that can efficiently track the name combinations and map them to counts which are updated as more matches are found. Reinventing that data structure is painful and unlikely to result in meaningful improvements.

You could try to collect NameGroups directly in the map instead of going via a count, but most approaches for that would, again, be more expensive than what you have now, and certainly much more awkward.

Honestly: what you have now is perfectly good, and not inefficient in any ways that are important. You should almost certainly stick to what you have.

Upvotes: 2

Related Questions