Mutability and Java 8's method references

Method references is a nice feature introduced in Java 8. It lets you make your lambdas even more concise:

// from...
someStream
  .map(item -> obj.someMethod(item))
  .moreStuff...

// to...
someStream
  .map(obj::someMethod)
  .moreStuff...

Some linting tools will even suggest you replace your lambdas with method references but, watch out, that sometimes have some unadverted consequences.

For instance, this is a piece of actual code I was working one:

private List<Application> applications() {
    return applications
            .stream()
            .map(name -> new Application().withName(name))
            .collect(Collectors.toList());
}

SonarLint suggested replacing the lambda with a method reference, and so I did without even thinking about it. I should have known better.

// This is how it looks after the "improvement"
private List<Application> applications() {
    return applications
            .stream()
            .map(new Application()::withName)
            .collect(Collectors.toList());
}

Here’s what Application#withName() looks like:

public Application withName(String name) {
    setName(name);
    return this;
}

By using a method reference this way, you are essentially creating a single Application instance, and applying the withName method to each element in the stream. Because withName just changes a mutable field in Application and returns the very same instance, the resulting list would contain multiple references to the same application where its name is the latest one applied.

Easier to understand if we rewrite it with this equivalent:

private List<Application> applications() {
    Application theOnlyApp = new Application();
    Function<String, Application> appFun = theOnlyApp::withName;
    return applications
            .stream()
            .map(appFun)
            .collect(Collectors.toList());
}

The code above is essentially the same, but you can clearly see that the method reference is just a function operating on the same instance (theOnlyApp).

In other words, the reason why this does not work as intended is because the closure passed to the map function encloses just the withName method, instead of the whole new Application().withName() chain.

This is one of the perils of mutability. Had Application been implemented as an immutable object this subtle bug would not have been possible. withName would instead return a new Application with a new name which is what we wanted.

Conclusion

• Do not obey your linter blindly. It feels good to have a friendly bot tell you what you can improve, but bots can and do make mistakes.

• Unit testing is paramount when refactoring your code. This would have been a nasty bug to encounter/fix if it had made it to production. Fortunately, that part of the code was covered by a test.

• Even though Java 8 brought functional paradigms easier to implement, Java is still Java and it’s very easy to shoot oneself on the foot.