Java lambda expression redundant call to external method -- how to rewrite?

Question

I'm trying to get used to using lambda expressions, but I frequently get caught up against something basic like this:

public List<Location> findAllAccessByUser(User user) {
        return listDao.getAccessList(user).stream()
            .filter(list -> findBySubOrgId(list.getOwnerOrg().getId()).isPresent())
            .map(list -> findBySubOrgId(list.getOwnerOrg().getId()).get())
            .collect(Collectors.toList());
    }

This method

Gets a list of AccessList objects,
reads the subOrgId property from each object
uses the findBySubOrgId() method to return an (Optional) Location object.
Collect the objects into a list of Location objects.

Since the findBySubOrgId() returns an Optional which may not be present, I figure I need to filter it so the return List doesn't contain any empty elements. But then I have the repeat call to the same method, which seems wasteful.

Ordinarily I'd assign it to a variable and reuse that, but I can't find a reference to how to do this with lambdas -- or if it's even necessary.

Alternately I could put a conditional in the map() expression instead, but again I'm not sure how to do this and be sure to remove nulls.

As written it works exactly as I expect, but I always try to optimize my code, and it never hurts to learn the right way to do things. What is the "correct" way to rewrite this? Or is it fine as is?

map to the Optional first, then filter out those that are not present, then map to their values. (Or use orElse(null) and filter out null values to skip a map step.) — Sotirios Delimanolis
– Sotirios Delimanolis, Commented Dec 23, 2016 at 1:24

shmosel · Accepted Answer · 2016-12-23 01:32:18Z

2

return listDao.getAccessList(user).stream()
        .map(list -> findBySubOrgId(list.getOwnerOrg().getId()))
        .filter(Optional::isPresent)
        .map(Optional::get)
        .collect(Collectors.toList())

answered Dec 23, 2016 at 1:32

shmosel

51k7 gold badges81 silver badges148 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Andreas Over a year ago

For a pure method reference version, replace first map() call with map(XX::getOwnerOrg).map(XX::getId).map(this::findBySubOrgId), where the two XX are the appropriate class names, and assuming findBySubOrgId() is non-static.

Collectives™ on Stack Overflow

Java lambda expression redundant call to external method -- how to rewrite?

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related