Is it a bad practice to throw exceptions in java 8 optional filters

Question

I wanted to leverage java 8 optional to validate values of the an object received (as a response). I'm curious to know, if it is a bad practice to do as below.

Optional.ofNullable(response)
.map(Response::getStatus)
.filter(status -> {
    if (status == Status.REJECTED)
        throw new RequestRejectedException("some exception");
    else if (status == Status.LOCKED)
        throw new ResourceLockedException("some other exception");
    return true;
})
.orElse(Status.UNAVAILABLE);

Wanted to know, if this is acceptable to write something like above or if there is a better way to do it, please suggest.

Using Exception for control flow is a hot topic, some people say that's a bad practice other say that's good because is more simple... My advice, if you use in your code exception like control flow use it, if not try use a return false. — Raúl Garcia, Aug 26 '19 at 11:54
__[the main point of `Optional` is to provide a means for a function returning a value to indicate the absence of a return value](https://stackoverflow.com/a/23464794/8198056) !__ — Adrian, Aug 26 '19 at 12:00
@Adrian I believe I understand the main objective of Optional, but my question is more towards validating value wrapped around by the Optional using filter() and if it is a violation of anything if an exception is thrown from there. The link you presented doesn't actually answer my question. — user3495691, Aug 26 '19 at 12:36
`status == 'rejected'` is not valid Java, even if `status` was a `String`, but it looks like `Status` is an `enum`… — Holger, Aug 26 '19 at 12:45
Why are you calling `.orElse(Status.UNAVAILABLE);` at the end? Are you actually using the returned status? — Holger, Aug 26 '19 at 12:58
I think that you're misusing the `filter` method. `ifPresent` seems more appropriate, since it's meant to process the value inside the Optional. — Arnaud Claudel, Aug 26 '19 at 13:02
@ArnaudClaudel `ifPresent` would be OK as well if the OP didn't set `Status.UNAVAILABLE` — Andrew Tobilko, Aug 26 '19 at 14:04

score 4 · Accepted Answer · answered Aug 26 '19 at 12:51

4

No, it's not OK.

return true;

You don't filter anything, do you? It would be better to handle exceptions after the status is known.

final String status = Optional.ofNullable(response)
                              .map(Response::getStatus)
                              .orElse(Status.UNAVAILABLE);

if ("rejected".equals(status)) {
    throw new RequestRejectedException("some exception");
}

Judging from your comment, you seem to understand what Optional is for. No need to warn you it's an inappropriate usage.

answered Aug 26 '19 at 12:51

Andrew Tobilko

48,120
14
91
142

Thanks for the response. Yes, I thought it would be inappropriate but wanted to make sure it is indeed. Now extending this question, I need to verify few other fields in the response object (and they too need to throw exceptions differently if unacceptable values are returned and then they would be handled appropriately by the exception handler). My initial thought was to chain up the filters for each field validations (like filter(validateStatus()).filter(validateType()) etc). But it seems like I'm better off fetching each field and validate them separately. Correct? – user3495691 Aug 26 '19 at 13:24
@user3495691 `filter(validateStatus())` just doesn't look right. Compare it with `filter(canBeValidated())` or `filter(isValid())`. "fetching each field and validating them" seems simpler, cleaner, and more reasonable. – Andrew Tobilko Aug 26 '19 at 13:31
What if he used `peek` instead of `filter`? – Vince Aug 26 '19 at 13:44
@VinceEmigh I thought about it. First, there is no `peek` for `Optional`. Second, `peek` is mainly used for debugging the flow. – Andrew Tobilko Aug 26 '19 at 13:47

Is it a bad practice to throw exceptions in java 8 optional filters

1 Answers1