8

I understand you can't return from a ifPresent() so this example does not work:

public boolean checkSomethingIfPresent() {
    mightReturnAString().ifPresent((item) -> {
        if (item.equals("something")) {
            // Do some other stuff like use "something" in API calls
            return true; // Does not compile
        }
    });
    return false;
}

Where mightReturnAString() could return a valid string or an empty optional. What I have done that works is:

public boolean checkSomethingIsPresent() {
    Optional<String> result = mightReturnAString();

    if (result.isPresent()) {
        String item = result.get();
        if (item.equals("something") {
            // Do some other stuff like use "something" in API calls
            return true;
        }
    }
    return false;
}

which is longer and does not feel much different to just checking for nulls in the first place. I feel like there must be a more succinct way using Optional.

Naman
  • 27,789
  • 26
  • 218
  • 353
Friedrich 'Fred' Clausen
  • 3,321
  • 8
  • 39
  • 70

5 Answers5

5

I think all you're looking for is simply filter and check for the presence then:

return result.filter(a -> a.equals("something")).isPresent();
Naman
  • 27,789
  • 26
  • 218
  • 353
  • Both your answer and the one by @ravindra-ranwala seem to work - not sure which is most suited to this use case. – Friedrich 'Fred' Clausen Feb 26 '19 at 03:50
  • 1
    @FredClausen They work, but can you do "*other stuff like API calls*" using these? – Kartik Feb 26 '19 at 03:52
  • @FredClausen Well, if all you wanted was to check for a condition, `filter` with `Predicate` would be apter as `map` on the other hand should be ideal when you're transforming the object. The `map` with predicate would just produce a `Boolean` after all. – Naman Feb 26 '19 at 03:53
  • Ah yeah, trying these suggestions out in the full project where I need them. Will update based on that. – Friedrich 'Fred' Clausen Feb 26 '19 at 03:53
  • @Kartik Some of that would depend on the other stuff definition as well. If it's just a stateless execution, `return result.filter(a -> { if (a.equals("something")) { //do some stuff return true; } return false; }).isPresent();` can also work. Really depends on what I could say from past. – Naman Feb 26 '19 at 03:55
  • 4
    Instead of `a -> a.equals("something")` you can also use `"something"::equals`. I’d not put the “other stuff” in here, as you could still chain it as an independent operation or just perform it when this check returned `true`. In the latter case, there would be no access to the value, but your don’t need it anyway when the value has been proven to be `"something"`. – Holger Feb 26 '19 at 09:11
3

How about mapping to a boolean?

public boolean checkSomethingIfPresent() {
    return mightReturnAString().map(item -> {
        if (item.equals("something")) {
            // Do some other stuff like use "something" in API calls
            return true; // Does not compile
        }
        return false; // or null
    }).orElse(false);
}
shmosel
  • 49,289
  • 6
  • 73
  • 138
2

While @nullpointer and @Ravindra showed how to merge the Optional with another condition, you'll have to do a bit more to be able to call APIs and do other stuff as you asked in the question. The following looks quite readable and concise in my opinion:

private static boolean checkSomethingIfPresent() {
    Optional<String> str = mightReturnAString();
    if (str.filter(s -> s.equals("something")).isPresent()) {
        //call APIs here using str.get()
        return true;
    }
    return false;
}

A better design would be to chain methods:

private static void checkSomethingIfPresent() {
    mightReturnFilteredString().ifPresent(s -> {
        //call APIs here
    });
}

private static Optional<String> mightReturnFilteredString() {
    return mightReturnAString().filter(s -> s.equals("something"));
}

private static Optional<String> mightReturnAString() {
    return Optional.of("something");
}
Kartik
  • 7,677
  • 4
  • 28
  • 50
1

The ideal solution is “command-query separation”: Make one method (command) for doing something with the string if it is present. And another method (query) to tell you whether it was there.

However, we don’t live an ideal world, and perfect solutions are never possible. If in your situation you cannot separate command and query, my taste is for the idea already presented by shmosel: map to a boolean. As a detail I would use filter rather than the inner if statement:

public boolean checkSomethingIfPresent() {
    return mightReturnAString().filter(item -> item.equals("something"))
            .map(item -> {
                // Do some other stuff like use "something" in API calls
                return true; // (compiles)
            })
            .orElse(false);
}

What I don’t like about it is that the call chain has a side effect, which is not normally expected except from ifPresent and ifPresentOrElse (and orElseThrow, of course).

If we insist on using ifPresent to make the side effect clearer, that is possible:

    AtomicBoolean result = new AtomicBoolean(false);
    mightReturnAString().filter(item -> item.equals("something"))
            .ifPresent(item -> {
                // Do some other stuff like use "something" in API calls
                result.set(true);
            });
    return result.get();

I use AtomicBoolean as a container for the result since we would not be allowed to assign to a primitive boolean from within the lambda. We don’t need its atomicity, but it doesn’t harm either.

Link: Command–query separation on Wikipedia

Ole V.V.
  • 81,772
  • 15
  • 137
  • 161
0

By the way if you really want to get value from Optional, use:

Optional<User> user = service.getCurrentUset();

return user.map(User::getId);
Ivan
  • 2,316
  • 2
  • 24
  • 22