Rewrite service request using optionals

Question

I want to rewrite the code below using Optionals ( I do not control jpaConnector ):

public boolean deleteLockStatus() {

    IMdss service = jpaConnector.getMdssService();
    if ( service == null ) {
        return false;
    }

    ServiceResponse response = null;
    try {
        response = service.deleteLockStatus();
    } catch (Exception e) {
        e.printStackTrace();
    }

    if ( response == null ) {
        return false;
    }
    if ( response.isError() ) {
        return false;
    }
    return true;
}

I have achievied this so far:

public boolean deleteLockStatus() {

    Optional<IMdss> service = Optional.ofNullable(jpaConnector.getMdssService());

    if (!service.isPresent()) { return false; }


    Optional<ServiceResponse> response = Optional.empty();
    try {
        response = Optional.ofNullable(service.get().deleteLockStatus());
        if ( response.isPresent() == false || response.get().isError() ) {
            return false;
        }
    } catch (Exception e) {
        e.printStackTrace();
        return false;
    }

    return true;
}

Is there a better and more native java 8 way? Thank you!!!

Michael · Accepted Answer · 2017-12-15T15:37:39.107

4

We start with an Optional<Service>, flat map that to an Optional<ServiceResponse> (using the regular map function would give us Optional<Optional<ServiceResponse>>), then map that to an Optional<Boolean>.

The Optional<Boolean> represents success or failure of the response. If we don't have a value here, an exception was thrown so we return false with orElse(false).

It's a shame about the checked exception and having to print the stack trace, or else it could be a lot more concise.

public boolean deleteLockStatus() {
    return Optional.ofNullable(jpaConnector.getMdssService())
        .flatMap(service -> {
            try {
                return Optional.ofNullable(service.deleteLockStatus());
            }
            catch(Exception e) {
                e.printStackTrace();
                return Optional.empty();
            }
        })
        .map(ServiceResponse::isError)
        .orElse(false);
}

Side note: catching Exception is usually a bad idea. You should be as specific as possible. Consider using this syntax if there are multiple possible exceptions which may be thrown.

As mentioned in the comments by Federico, you can replace the flatMap with this slight simplification if you don't mind using null. I would personally prefer the version above.

.map(service -> {
     try {
         return service.deleteLockStatus();
     }
     catch(Exception e) {
         e.printStackTrace();
         return null;
     }
 })

edited Dec 15 '17 at 15:37

answered Dec 15 '17 at 10:14

Michael

41,989
11
82
128

Thanks, wow answer in 1min O.O ... I don't have to print the stackTrace, but I need to handle the exception ... but I guess that only saves me one line!? – Shakka Dec 15 '17 at 10:19
1

@Shakka You could consider moving the contents of the `flatMap` lambda to its own function and then replacing it with a method reference but it's up to you to decide whether that would be more readable. – Michael Dec 15 '17 at 10:26
i fixed an small error the last bracket should be a semicolon – Shakka Dec 15 '17 at 10:29
Why are you using `flatMap`? A common `map` would suffice, and you wouldn't need to wrap the response of `service.deleteLockStatus()` into a new `Optional`. – fps Dec 15 '17 at 15:13
Just return `null` – fps Dec 15 '17 at 15:25
1

Well, this is a matter of opinion and debate, I think. I always try to not use `null` either, but if I'm forced to use `flatMap` instead of `map`, and to wrap the result of a service call in a new `Optional` and to return an empty `Optional` from within the `catch` block, all this to just not use one `null`... But I understand your point. Anyways, I have upvoted your answer, because it's correct. – fps Dec 15 '17 at 15:32
1

@FedericoPeraltaSchaffner Neither is objectively better. I didn't think about your way when writing my solution originally - I have strong habits of avoiding null. I've added it as an option anyway. – Michael Dec 15 '17 at 15:40
2

Michael, thank you very much for including it in the edit. Now future readers will have both ways at their disposal and will choose the one they most like. This makes your answer even better, because it's as objective as possible. – fps Dec 15 '17 at 15:46

Rewrite service request using optionals

1 Answers1