Summary

The article discusses the potential pitfalls of using Java Stream's forEach() method, advocating for a separation of concerns and adherence to functional programming principles for better code testability and maintainability.

Abstract

The article "Is .stream().map().forEach() an Antipattern?" delves into the use of Java Stream's forEach() method, questioning whether its use violates functional programming principles. The author, Emanuel Trandafir, argues that Java developers may not fully grasp functional patterns, leading to the misuse of streams, particularly with the forEach() method, which introduces side effects and imperative programming styles. The article presents a refactoring approach to improve the testability of business logic by separating queries (pure functions without side effects) from commands (functions that cause side effects). This is achieved by splitting a stream pipeline into distinct functions: finding employees by ID, filtering those eligible for promotion, and sending notifications to HR. The refactoring aims to decouple the business logic from the notification action, thus simplifying testing by reducing the need for mocks and enabling the use of pure functions.

Opinions

The author suggests that Java developers might hastily adopt functional patterns without a deep understanding of their principles, such as pure functions, immutability, and command-query separation.
The forEach() method is seen as a source of side effects in an otherwise functional pipeline, which can lead to less maintainable and testable code.
The article promotes the Command Query Separation Principle, emphasizing that functions should either be commands (causing side effects) or queries (pure functions without side effects).
Testability is a key concern, with the author advocating for refactoring to reduce dependencies and the need for mocks in tests.
The author suggests that methods should not be responsible for both retrieving data and causing side effects, proposing that the notifyHrAboutEmployeesEligibleForPromotion() method should ideally accept a list of employees rather than IDs to decouple it from the repository.
Pure functions are highly valued for their ease of testing without the need for complex setups or mocks.
The article encourages further refactoring for complex logic, such as extracting it into a separate class or making it a public static method if it remains simple.
The author emphasizes the importance of testing over strict encapsulation in certain cases, especially when dealing with legacy code.
Emanuel Trandafir invites readers to engage with the content, provide feedback, and explore his other articles on clean code, design, and functional programming.

Is .stream().map().forEach() an Antipattern?

Let’s discuss Java Stream’s forEach() method: is it violating Functional Programming principles?

Java developers are not typically familiar with functional patterns, but since the introduction of Java 8, their popularity has increased. Though, I feel that sometimes we are too quick to use functional pipelines without fully comprehending functional programming principles.

It might take some time and effort to fully understand and embrace concepts such as pure functions, immutability, and Command-Query separation.

Let’s assume we need a method that takes in a list of employee IDs and sends a notification to the HR department for those employees who are eligible for promotion:

public void notifyHrAboutEmployeesEligibleForPromotion(List<Long> employeeIds) {
    employeeIds.stream()
        .map(employeeRepository::findById)
        .filter(employee -> employee.role() == Role.MIDDLE)
        .filter(employee -> matchesHoursOfServiceCriteria(employee.hiringDate()))
        .forEach(notificationService::notifyHrDepartment);
}

Even though the method is small and it might look alright, it has some design smells.

The Problem With forEach()

We can see that the forEach line is the only one that introduces side effects into our code. This line represents the point where our code transitions out of the functional paradigm and into a more imperative style of programming. At the same time, it couples our filtering logic to this notificationService, making it harder to test.

To test this method, we would potentially need two mocks: Initially, we will need to mock the employeeRepository and return some test data when the findById method is called. After that, we will need to mock the notificationService and verify the invocations of the notifyHrDepartment method.

The goal of our refactoring will be to improve the testability of the code containing the business logic, in our case, the logic to determine if an employee is eligible for a promotion.

Command Query Separation Principle

According to the Command Query Separation Principle, all our functions should be either commands or queries. A command is a “void” method that causes side effects (changes state, doing REST calls, DB operations.. etc).

On the other hand, a query is retrieving data without causing any side effects. Ideally, we would like our queries to be pure functions, which means that they always return the same results when called with the same arguments, regardless of how many times they are called.

Consequently, let’s follow this principle and split our pipeline into three different functions:

we find the employees based on the given IDs // this will be a query, but not a pure function because it depends on the repository;
we filter them and only keep those who meet the promotion criteria // this will be a query and also a pure function;
for each of them, we call the function that sends a notification // this will be a command;

public void notifyHrAboutEmployeesEligibleForPromotion(List<Long> employeeIds) {
    List<Employee> employees = findEmployees(employeeIds);

    List<Employee> employeesEligibleForPromotion = filterEmployeesEligibleForPromotion(employees);

    sendNotificationToTheHr(employeesEligibleForPromotion);
}

private List<Employee> findEmployees(List<Long> employeeIds) {
    return employeeIds.stream()
        .map(employeeRepository::findById)
        .toList();
}

private List<Employee> filterEmployeesEligibleForPromotion(List<Employee> employees) {
    return employees.stream()
        .filter(employee -> employee.role() == Role.MIDDLE)
        .filter(employee -> matchesHoursOfServiceCriteria(employee.hiringDate()))
        .toList();
}

private void sendNotificationToTheHr(List<Employee> employeesEligibleForPromotion) {
    employeesEligibleForPromotion
        .forEach(notificationService::notifyHrDepartment);
}

At this point, we need to make an important decision: do we even need the first function? Should notifyHrAboutEmployeesEligibleForPromotion() be responsible for retrieving the employees from the repository? Would it be acceptable to make it accept a list of employees instead of a list of IDs?

If the answer is “yes”, we can further simplify the design and decouple our method from the employeeRepository. As a result, we eliminate one of the “mocks” from our test:

public void notifyHrAboutEmployeesEligibleForPromotion(List<Employee> employees) {
    List<Employee> employeesEligibleForPromotion = filterEmployeesEligibleForPromotion(employees);

    sendNotificationToTheHr(employeesEligibleForPromotion);
}

private List<Employee> filterEmployeesEligibleForPromotion(List<Employee> employees) {
    return employees.stream()
        .filter(employee -> employee.role() == Role.MIDDLE)
        .filter(employee -> matchesHoursOfServiceCriteria(employee.hiringDate()))
        .toList();
}

private void sendNotificationToTheHr(List<Employee> employeesEligibleForPromotion) {
    employeesEligibleForPromotion
        .forEach(notificationService::notifyHrDepartment);
}

Testing The Code That Matters

Moreover, filterEmployeesEligibleForPromotion —the method where we keep most of the logic for this use case — is now a pure function. This can be tested very easily and fast: no mocks, no beans, just plain java code. I know, I know, the method is private now, but there are some ways of making it available for tests:

If it grows in complexity, we can extract it into a separate class with a public method and test easily test it with many combinations of inputs and expected outputs. (check out JUnit5’s @ParameterizedTests)
On the Other hand, if it stays small and simple, we can even make it a public static (helper) method: it is a pure function, it does not depend on anything and it is not changing any state.
Lastly, we can make it package-protected and test it directly. As Uncle Bob says: “test trumps encapsulation”! This technique is often used for legacy code where we don’t want to go into deep refactoring.

Conclusion

In this article, we discussed Java Stream’s forEach method. We learned that forEach is creating side effects and it is not allowing us to write code in a pure, functional style.

We discovered that, if we have business logic that we want to test, we can split the query from the command by collecting the stream into a list and only calling the forEach in a second step.

Thank You!

Thanks for reading the article and please let me know what you think! Any feedback is welcome.

If you want to read more about clean code, design, unit testing, functional programming, and many others, make sure to check out my other articles. Do you like the content? Consider following or subscribing to the email list.

Finally, if you consider becoming a Medium member and supporting my blog, here’s my referral.

Happy Coding!