Optional monad and the Law of Demeter in Java

Question

while I was reviewing some code, I came across this snippet.

List<User> users = /* Some code that initializes the list */;
users.stream()
     .filter(user -> user.getAddress().isPresent())
     .map(/* Some code */)
// And so on...

The call of method user.getAddress() returns an Optional<Address>. Following the famous Law of Demeter (LoD), the above code is not clean. However, I cannot figure out how to refactor it to make it cleaner.

As a first attempt could be to add to the User class a method hasAddress(), but this method overcomes the need of having an Optional<Address>, IMO.

How should I refactor the above code? In this case, is it worth to satisfy the LoD?

EDIT: I missed specifying that in the map method I do not want to return the address.

Answer 1

Well, you summarized it yourself pretty well: If you wanted to couple more loosely by introducing hasAddress(), why return an Optional.

Reading into what the LoD says, it talks about having "limited" knowledge about "closely related" units. Sounds like a gray area to me, but going further it also mentions the "Only one dot" rule. Still, I would agree with the comments to your question that a null check (or isPresent()) is entirely fine (heck, a real null check technically doesn't even need a dot ;P ).

If you wanted to really encapsulate more, you could remove the getAddress() completely and instead offer:

class User {
    private Optional<Address> address;

    boolean hasAddress() {
        return address.isPresent();
    }

    // still exposes address to the consumer, guard your properties
    void ifAddressPresent(Consumer<Address> then) {
        address.ifPresent(then::accept);
    }

    // does not expose address, but caller has no info about it
    void ifAddressPresent(Runnable then) {
        address.ifPresent(address -> then.run());
    }

    // really keep everything to yourself, allowing no outside interference
    void ifAddressPresentDoSomeSpecificAction() {
        address.ifPresent(address -> {
            // do this
            // do that
        });
    }
}

But again, as the commenters pointed out: Is it worth it/necessary? All these laws/principles are rarely absolute and more guidelines than dogmas. In this case it might be about balancing LoD vs. KISS.

---

In the end, it is up to you to decide whether this specific example benefits from moving the functionality of your stream into the User class. Both are valid, and readability/maintainability/cleanliness depend on:

• the specific case
• how exposed this code is to other modules
• the number of delegate methods you would need in the User class
• your architecture (If you are in a UserDao class for example, do you really want to move database access your User POJO class? Isn't the DAO made for exactly this purpose? Does this qualify as "closely related" and would allow a violation of the "Only one dot" rule?)
• ...

Answer 2

Don't know if it's cleaner (as you need to use the fact that getAddress returns an optional anyway), but in Java 9 you can do:

users.stream()
    .map(User::getAddress)
    .flatMap(Optional::stream)
    .map(/* Some code */)

or

users.stream()
    .flatMap(user -> user.getAddress().stream())
    .map(/* Some code */)