Java 8 Optional / functional code optimization

Question

There is a method that receives "someObj" and its purpose is to check the var ASet of type Set<>, iterate through it and replace its objects with the database object. For that I written the following code:

if(!CollectionUtils.isEmpty(someObj.getASet())){
        someObj.setASet(
            someObj.getASet()
                    .stream()
                    .map( c -> AService.getExistingA(c.getId()))
                    .collect(Collectors.toSet())
       );
    }

It does the purpose but I'm really not happy with it's readability.

 Optional.ofNullable(someObj.getASet())
            .ifPresent( ASet->  someObj.setASet(
                                                ASet.stream()
                                                .map( c -> AService.getExistingA(c.getId()))
                                                .collect(Collectors.toSet())
            ));

Now it looks even less readable, can you recommend a better way? I think the worst problem is that someObj.setASet, it simply looks weird, is ther any functional way to replace that object after the collect?

Brian Goetz · Accepted Answer · 2017-01-16T15:48:38.433

6

Try using Optional.map:

Optional.ofNullable(someObj.getASet())
        .map(set -> set.stream()
                       .map(c -> AService.getExistingA(c.getId()))
                       .collect(Collectors.toSet()))
        .ifPresent(set -> someObj.setASet(set));

Now, the transform logic and the conditional imperative "set the result" are separate.

Or, the old standby works too -- no shame in using it:

Set set = someObj.getASet();
if (set != null) {
    Set newSet = set.stream()
                    .map(c -> AService.getExistingA(c.getId()))
                    .collect(Collectors.toSet());
    someObj.setASet(newSet);
}

edited Jan 16 '17 at 15:48

answered Oct 19 '16 at 14:26

Brian Goetz

90,105
23
150
161

In my head that would break so I just read the documentation on Map inside optional: If a value is present, apply the provided mapping function to it, and if the result is non-null, return an Optional describing the result. Otherwise return an empty Optional. I love it, big thx – Als Oct 19 '16 at 14:41
3

But a much better alternative is to fix the API design. If `getASet()` never returns `null`, but just an empty set, the whole `Optional`/conditional logic becomes unnecessary. – Holger Oct 19 '16 at 15:57
@Holger that is a good idea but I'm using lombok to auto generate the getters and setters so that would kinda break all my code, still that is a really good idea, maybe on my next fresh project I'll do that – Als Oct 20 '16 at 10:29
@Als: I’m sure that the auto-generated code doesn’t introduce `null`s, so when a getter returns `null`, the property itself is `null`, which should be prevented in the first place. Likewise, a property of a collection type should not consist of a reference to an object potentially being mutated somewhere else, so a setter should always guard by copying the argument, if it’s a mutable collection, and either, reject `null` or substitute it with an empty collection. If the auto-generated code doesn’t provide this, but mindlessly stores whatever reference it gets, you shouldn’t use that generator… – Holger Oct 20 '16 at 10:39

Java 8 Optional / functional code optimization

1 Answers1