Rewrite if-statement using Optional map and filter

Question

I have a predicate which accepts Optional<LocalDateTime> and I want to check if it is present and the LocalDateTime is before current date.

I can write it with if statements which would look like this:

@Override
public boolean test(Optional<ResetPassword> resetPassword) {
    if (resetPassword.isPresent()) {
        if (!resetPassword.get().getValidUntil().isBefore(LocalDateTime.now())) {
            throw new CustomException("Incorrect date");
        }
        return true;
    }
    return false;
}

How could I rewrite this using the Optional.map and Optional.filter functions?

`.map` and `.filter` are for streams, which usually comes from a collection. There is no point in using these in a non-collection object. — KarelG, Oct 31 '18 at 08:45
You should not use `Optional` as a parameter to anything. It's meant to be used only as a return type. — marstran, Oct 31 '18 at 08:50
Well, you can do that after a collection stream to have the same semantics at the end of the chain (eg after `.findAny()`) to convert the object into another type if desired. But outside that? It is also strange that OP is passing an `Optional` as a parameter. I only use it as return. — KarelG, Oct 31 '18 at 08:51
@marstran *"You should never use an Optional as a parameter to anything."* As an unqualified assertive statement this is just plain wrong. It's a matter of style. Having an optional as a parameter type makes it clear to readers that the parameter might be null, which makes the code easier to work with. — Lii, Oct 31 '18 at 09:18
@LiolikasBolikas: There doesn't seem to be a good way to solve your problem with `map` and `filter`. I think your solution is about as good as it's possible to make it. The reason why it is hard is, I think, it that the combination of results that you want doesn't translate well to the logic of `Optional`. In your case you want to work with `boolean`, `ResetPassword` and exception, and that doesn't really fit the model. I think... — Lii, Oct 31 '18 at 09:28
@Lii It's certainly not an unqualified statement. It is a statement that originates with Brian Goetz himself. See https://stackoverflow.com/a/26328555/4137489. — marstran, Oct 31 '18 at 10:19
I've also argued against the use of `Optional` as a method parameter. If this is intended to be used as a `Predicate`, though, then its usage in this case might be warranted. This would let you take a stream of `Optional` and filter them using this predicate. While semantically this particular case seems a bit odd, there might be other cases where it's useful. More concerning is the fact that this method throws an exception in some cases, which makes it entirely unsuitable for most uses as a `Predicate`. — Stuart Marks, Oct 31 '18 at 16:10
The purpose of this predicate was to validate ResetPassword object if it's present and if its date time is before current date time, and if not i just throw exception which is handled by Global exception handler, the reason i added it to Predicate is to just bundle few predicates, iterate over them and check rules so there would be no multiple if statements. What you would suggest for me to change in this case ? — LiolikasBolikas, Nov 02 '18 at 14:40

score 1 · Accepted Answer · answered Oct 31 '18 at 08:55

1

You should never use an Optional as a parameter to anything. Instead, you should let your function take a ResetPassword, and only call it if the value of the Optional is present. Like this:

public void test(ResetPassword resetPassword) {
    if (!resetPassword.getValidUntil().isBefore(LocalDateTime.now())) {
        throw new CustomException("Incorrect date");
    }
}

And then call it like this:

resetPasswordOptional
    .ifPresent(rp -> test(rp));

answered Oct 31 '18 at 08:55

marstran

26,413
5
61
67

3

*"You should never use an Optional as a parameter to anything."* As an unqualified assertive statement this is just plain wrong. It's a matter of style. Having an optional as a parameter type makes it clear to readers that it might be null. – Lii Oct 31 '18 at 09:09
2

@Lii It's certainly not an unqualified statement. It is a statement that originates with Brian Goetz himself. See https://stackoverflow.com/a/26328555/4137489. – marstran Oct 31 '18 at 10:17

Garik Kalashyan · Answer 2 · 2018-10-31T09:44:50.543

0

I hope this one will help you, in addition, pay attention to the exception which one you throw if it is RuntimeException your app will crash in case of a false condition.

 public boolean test(Optional<ResetPassword> resetPassword) {
        return resetPassword.isPresent() && resetPassword
                .map(ResetPassword::getValidUntil)
                .filter(localDateTime -> localDateTime.isBefore(LocalDateTime.now()))
                .orElseThrow(() -> new CustomException("Incorrect date")) != null;
    }

edited Oct 31 '18 at 09:44

answered Oct 31 '18 at 09:02

Garik Kalashyan

129
1
4

That should work! I think what the poster already has is clearer, but this is shorter and at least it uses `map` and `filter`! (If that's your goal...) – Lii Oct 31 '18 at 09:49

Rewrite if-statement using Optional map and filter

2 Answers2