Java Stream API с цепочкой не чистых функций

Question

Предположим, у меня есть класс Person

public class Person {
    private final String name;
    private final int age;
    private boolean rejected;
    private String rejectionComment;

    public void reject(String comment) {
        this.rejected = true;
        this.rejectionComment = comment;
    }

    // constructor & getters are ommited
}

и мое приложение выглядит примерно так

class App {
    public static void main(String[] args) {
        List<Person> persons = Arrays.asList(
            new Person("John", 10),
            new Person("Sarah", 20),
            new Person("Daniel", 30)
        )

        persons.forEach(p -> {
            rejectIfYoungerThan15(p);
            rejectIfNameStartsWithD(p);
            // other rejection functions
        }
    }

    private static void rejectIfYoungerThan15(Person p) {
        if (!p.isRejected() && p.getAge() < 15) {
            p.reject("Too young")
        }
    }

    private static void rejectIfNameStartsWithD(Person p) {
        if (!p.isRejected() && p.getName().startsWith("D")) {
            p.reject("Name starts with 'D'")
        }
    }

    // other rejection functions
}

Дело в том, что мне не нравится, что я должен выполнять !p.isRejected() проверку каждой функции отклонения. Более того, нет смысла передавать уже отклоненного человека следующим фильтрам. Поэтому моя идея состоит в том, чтобы использовать механизм Stream.filter и сделать что-то вроде

persons.stream().filter(this::rejectIfYoungerThan15).filter(this::rejectIfNameStartsWithD)...

И изменить сигнатуру для этих методов, чтобы она возвращала true, если переданный Person не был отклонен, и false в противном случае.

Но мне кажется, что очень плохо использовать filter с не чистыми функциями.

У вас есть идеи, как сделать это более элегантно?

Answer 1

Когда вы изменяете функции проверки только для проверки состояния (т.е. не для вызова p.isRejected()) и возврата boolean, вы уже сделали необходимые шаги для короткого замыкания:

private static boolean rejectIfYoungerThan15(Person p) {
    if(p.getAge() < 15) {
        p.reject("Too young");
        return true;
    }
    return false;
}

private static boolean rejectIfNameStartsWithD(Person p) {
    if(p.getName().startsWith("D")) {
        p.reject("Name starts with 'D'");
        return true;
    }
    return false;
}

можно использоватьas

   persons.forEach(p -> {
        if(rejectIfYoungerThan15(p)) return;
        if(rejectIfNameStartsWithD(p)) return;
        // other rejection functions
    }
}

Операция Stream * filter не будет делать ничего, кроме проверки возвращенного значения boolean и восстановления.Но в зависимости от фактической работы терминала Stream короткое замыкание может пойти еще дальше и в итоге не проверить все элементы, поэтому вам не следует вводить операцию Stream здесь.

Answer 2

Вы должны сделать Person неизменным и позволить методам отклонения вернуть новый Person. Это позволит вам связать map звонки. Примерно так:

public class Person {
    private final String name;
    private final int age;
    private final boolean rejected;
    private final String rejectionComment;

    public Person reject(String comment) {
        return new Person(name, age, true, comment);
    }

    // ...

}

class App {

    // ...

    private static Person rejectIfYoungerThan15(Person p) {
        if (!p.isRejected() && p.getAge() < 15) {
            return p.reject("Too young");
        }
        return p;
    }
}

Теперь вы можете сделать это:

persons.stream()
       .map(App::rejectIfYoungerThan15)
       .map(App::rejectIfNameStartsWithD)
       .collect(Collectors.toList());

Если вы хотите удалить отклоненных лиц, вы можете добавить фильтр после сопоставления:

.filter(person -> !person.isRejected())

EDIT:

Если вам нужно закорачивать отклонения, вы можете объединить свои функции отклонения в новую функцию и остановить ее после первого отклонения. Примерно так:

/* Remember that the stream is lazy, so it will only call new rejections 
 * while the person isn't rejected.
 */
public Function<Person, Person> shortCircuitReject(List<Function<Person, Person>> rejections) {
    return person -> rejections.stream()
            .map(rejection -> rejection.apply(person))
            .filter(Person::isRejected)
            .findFirst()
            .orElse(person);
}

Теперь ваш поток может выглядеть так:

List<Function<Person, Person>> rejections = Arrays.asList(
    App::rejectIfYoungerThan15, 
    App::rejectIfNameStartsWithD);

List<Person> persons1 = persons.stream()
    .map(shortCircuitReject(rejections))
    .collect(Collectors.toList());

Answer 3

Вызов этих методов из лямбды - это нормально, однако для лучшей читабельности вы можете переименовать эти методы, чтобы показать, что они делают, и вернуть boolean, например ::

private boolean hasEligibleAge(Person p){..}
private boolean hasValidName(Person p){..}

Другим подходом было бы заключить эти методы в другой метод (чтобы отразить бизнес-логику / поток), например ::10000*

private boolean isEligible(Person p){
    //check age
    //check name
}