0

I have a method findUrl() that take a User and a Permission as parameters and queries the Database and implements some logic to find the URLs accessible to this User in comma separated string.

I want to develop a null-safe version of this method taking same User and Permission as parameters and then returning an Optional which will be empty if at least one of the values passed to it is empty or findUrl method return null. I wrote the below code but it seems like a null check based implementation and I don't see much benefit of using Optional to make code more concise.

public Optional<String> nullSafeFindUrl(User user, Permissions permissions) {
    if ((Optional.ofNullable(user)).isPresent() && (Optional.ofNullable(permissions)).isPresent()) {
        return Optional.ofNullable(findUrl(user.get(), permissions.get()));
    } else {
        return Optional.empty();
    }
}

Is there a better way to do it to make code more readable ?

Ankur Singh
  • 339
  • 3
  • 16
  • "but it more seems like null check based implementation" what exactly do you mean with this? – E_net4 Feb 23 '17 at 14:09
  • 5
    For the record: using Optionals as parameter is not exactly best practice; see http://stackoverflow.com/questions/31922866/why-should-java-8s-optional-not-be-used-in-arguments ... yes, crazy enough, you are only supposed to **return** Optional objects; but not to use them as parameters or even fields. – GhostCat Feb 23 '17 at 14:13
  • @E_net4 like here i am using if (user.isPresent() && permissions.isPresent()) to check if the value is present in Optional in same way like i'll be doing null check if these are not of Optional Type – Ankur Singh Feb 23 '17 at 14:16
  • 1
    If it is not a valid business case (to have both user and permissions null), best approach would be to throw a runtime exception when both are invalid. And get rid of Optional usage. – Andrei G Feb 23 '17 at 14:17
  • 3
    This isn’t `null`-safe. I can still invoke `nullSafeFindUrl(null, null)` and get a `NullPointerException` at runtime. So this design has no benefit, but makes calling this method more complicated… – Holger Feb 23 '17 at 14:55
  • @Holger edited the code to make it nullsafe – Ankur Singh Feb 23 '17 at 15:10
  • @GhostCat edited the code to remove Optional as parameters – Ankur Singh Feb 23 '17 at 15:10
  • 1
    @Ankur singh: Ok, and now ask yourself… is this simpler than `return user!=null && permissions!=null? Optional.ofNullable(findUrl(user, permissions)): Optional.empty();`? – Holger Feb 23 '17 at 15:25
  • @Holger That's what i have originally asked in the question because i don't see any benefit of using Optional here .But if you check the solution that i have added and that is much simpler – Ankur Singh Feb 23 '17 at 15:30
  • 2
    That’s why `Optional`s are preferred as *return types*. When implementing a method returning an `Optional`, you can make a guaranty to never return `null`. There is much lesser benefit within a method’s code flow or for parameters. – Holger Feb 23 '17 at 15:37

2 Answers2

2

What you need is to flatmap and map in one go.

return user.flatMap(u -> permissions.map(perm -> findUrl(u, perm));

This will return an empty Optional or an optional with the return value if both are valid

Ash
  • 2,562
  • 11
  • 31
2

The answer is similar to Ash but I'll add some explanation also so that it can help others

invoke flatMap on the first optional, so if this is empty, the lambda expression passed to it won’t be executed at all and this invocation will just return an empty optional.

Conversely, if the User is present, it uses it as the input of a Function returning an Optional as required by the flatMap method. The body of this function invokes a map on the second optional, so if it doesn’t contain any Permission, the Function will return an empty optional and so will the whole null- Safe method.

Finally, if both the User and the Permission are present, the lambda expression passed as argument to the map method can safely invoke the original findUrl method with them.

So after applying the above the implementation of method got reduced to single Line

public Optional<String> nullSafeFindUrl(User user, Permissions permissions) {
    (Optional.ofNullable(user)).flatMap(u -> (Optional.ofNullable(permissions)).map(p -> findUrl(p, u)));

}
Ankur Singh
  • 339
  • 3
  • 16