Compare values of elements in Stream

I'm having fun with Java's Stream library and lambdas.

The following code looks for persons within a list that have the same ID (which might indicate that something's wrong with the data) and prints out each group of people that share one ID.

I'm not sure if I'm doing this the most concise way, though.

This is the Person class:

public class Person {
    private String name;
    private String id;

    public Person(String name, String id) {
        this.name = name;
        this.id = id;
    }

    public String getId() {
        return id;
    }

    @Override
    public String toString() {
        return "Person [name=" + name + ", id=" + id + "]";
    }
}

This is the code I'd love to get feedback on:

// Set up test data
List people = Arrays.asList(new Person("Michael", "1"),
        new Person("Tobias", "2"), new Person("Nicole", "3"),
        new Person("Sarah", "3"));

// Group persons by their ID
Map> peopleById = people.stream().collect(
        Collectors.groupingBy(Person::getId));

// Print out groups of people that share one ID
peopleById
        .values()
        .stream()
        .filter(peopleWithSameId -> peopleWithSameId.size() > 1)
        .forEach(
                peopleWithSameId -> System.out
                        .println("People with identical IDs: "
                                + peopleWithSameId));

The code does what I want and I think it's readable but I doubt that this is the most I can get out of Java's functional features.

I wonder if you know a more elegant way to solve the problem.

Your code and Java 8 usage looks fine in general to me.

I do see an issue with the Person class, it looks like you are intending it to be an immutable class, if so, then you should also enforce it.

You need to ensure that the name and id fields can never be changed, you can do this by adding final to them. Your code currently seems to be safe, but it is not. I can extend Person and offer a method there to change the name and id fields, which violates the assumed variant of that those fields in Person are immutable.

Simply changing it to the following will do:

public class Person {
    private final String name;
    private final String id;
    ...
}

Onto the Java 8 usage now.

It is a good thing that you use the Collectors.groupingBy to provide a Map>, you cannot do it much faster either way if you want it to work with any kind of List as input and in this way you'll save yourself from nasty bugs and reimplementing what lots of people have already done, namely the grouping by operation.

Printing the offending values using Stream seems fine as well, except that you may rewrite it to look a little bit cleaner, something like this could work:

peopleById.values().stream()
    .filter(personList -> personList.size() > 1)
    .forEach(personList -> System.out.println("People with identical IDs: " + personList);

This is my personal preference on how to format it though, the only real change is to rename peopleWithSameId to personList, as it is simply a List and nothing more or less.

You've done a good job overall.

StackExchange Code Review Q#62815, answer score: 10