how use only higher-order functions to check if the return of web service is null using Optional and avoid noSuchElementException

Question

Context: I have a web service which return a family with its menbers. A family will always have a father and a mother and no child or multiple childreen. The service is described below by its wsdl.

Purpose: I want to use effectively the Optional from Java 8 and avoid classical way to check null. By classical, I mean the way we were used to accomplish until Java 7.

If I assume that the webservice will always return a family this would be enough:

@Test
public void test1() {
    Family f = helloWorldClientImplBean.allFamily();

    f.getChildren().stream().filter(x -> x.getFirstName().equalsIgnoreCase("John")).findFirst()
            .ifPresent(y -> System.out.println(y.getLastName()));
}

I tested and I could see that, as long as I get a family answered by service, it would perfectly works not matter if I have children or not. I mean, in the service implementation below, if I commented the olderSon and youngSon code there will be no null exception at all.

The problem raises when the service return null.

After read several blogs and discussion about it I reach this code which properly check if the return of service was null.

@Test
public void testWorkingButSeemsOdd() {

    //Family f = helloWorldClientImplBean.allFamily();
    Family f = null; //to make simple the explanation

    Optional<Family> optFamily = Optional.ofNullable(f);

    if (optFamily.isPresent()) {

        optFamily.filter(Objects::nonNull).map(Family::getChildren).get().stream().filter(Objects::nonNull)
                .filter(x -> x.getFirstName().equalsIgnoreCase("John")).findFirst()
                .ifPresent(y -> System.out.println("Optional: " + y.getLastName()));

    }

What would be more clean for me would be one of these approaches (all of them are failling but I believe they can show what I have been trying to do):

// here I try to filter if f is not null before mapping

@Test
public void testFilterNonNull() {
    Family f = null;
    Optional.ofNullable(f).filter(Objects::nonNull).map(Family::getChildren).get().stream().filter(Objects::nonNull)
            .filter(x -> x.getFirstName().equalsIgnoreCase("John")).findFirst()
            .ifPresent(y -> System.out.println(y.getLastName()));

}

I know the next doesn't compile but I guess it is possible to reach something similar

@Test
@Ignore
public void testOptionalNullable() {
    Family f = helloWorldClientImplBean.allFamily();

    Optional.ofNullable(f).orElse(System.out.println("Family is null")).map(Family::getChildren).get().stream().filter(Objects::nonNull)
            .filter(x -> x.getFirstName().equalsIgnoreCase("John")).findFirst()
            .ifPresent(y -> System.out.println(y.getLastName()));

}

wsdl

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<wsdl:definitions targetNamespace="http://codenotfound.com/services/helloworld"
    xmlns:soap="http://schemas.xmlsoap.org/wsdl/soap/" xmlns:tns="http://codenotfound.com/services/helloworld"
    xmlns:wsdl="http://schemas.xmlsoap.org/wsdl/" xmlns:xsd="http://www.w3.org/2001/XMLSchema"
    name="HelloWorld">

    <wsdl:types>
        <schema targetNamespace="http://codenotfound.com/services/helloworld"
            xmlns="http://www.w3.org/2001/XMLSchema" xmlns:tns="http://codenotfound.com/services/helloworld"
            elementFormDefault="qualified" attributeFormDefault="unqualified"
            version="1.0">

            <element name="family">
                <complexType>
                    <sequence>
                        <element name="father" type="tns:persontype" minOccurs="1"
                            maxOccurs="1" />
                        <element name="mother" type="tns:persontype" minOccurs="1"
                            maxOccurs="1" />
                        <element name="children" type="tns:persontype" minOccurs="0"
                            maxOccurs="unbounded" />
                    </sequence>
                </complexType>
            </element>

            <complexType name="persontype">
                <sequence>
                    <element name="firstName" type="xsd:string" />
                    <element name="lastName" type="xsd:string" />
                </sequence>
            </complexType>

            <element name="EmptyParameter" type="tns:voidType" />

            <complexType name="voidType">
                <sequence />
            </complexType>
        </schema>
    </wsdl:types>

    <!-- Message -->

    <wsdl:message name="emptyRequest">
        <wsdl:part name="emptyParameter" element="tns:EmptyParameter" />
    </wsdl:message>

    <wsdl:message name="allFamiliesResponse">
        <wsdl:part name="allFamiliesResponse" element="tns:family" />
    </wsdl:message>

    <!-- PortType -->

    <wsdl:operation name="allFamilies">
            <wsdl:input message="tns:emptyRequest" />
            <wsdl:output message="tns:allFamiliesResponse"></wsdl:output>
        </wsdl:operation>
    </wsdl:portType>

    <!-- Binding -->

    <wsdl:binding name="HelloWorld_Binding" type="tns:HelloWorld_PortType">
        <soap:binding style="document"
            transport="http://schemas.xmlsoap.org/soap/http" />

        <wsdl:operation name="allFamilies">
            <wsdl:input>
                <soap:body use="literal" />
            </wsdl:input>
            <wsdl:output>
                <soap:body use="literal" />
            </wsdl:output>
        </wsdl:operation>
    </wsdl:binding>

    <wsdl:service name="HelloWorld_Service">
        <wsdl:port name="HelloWorld_Port" binding="tns:HelloWorld_Binding">
            <soap:address location="http://localhost:9090/cnf/services/helloworld" />
        </wsdl:port>
    </wsdl:service>
</wsdl:definitions>

relevant part of service implementation:

@Override
public Family allFamilies(VoidType emptyParameter) {
    ObjectFactory factory = new ObjectFactory();
    Family result = factory.createFamily();
    Persontype father = new Persontype();
    father.setFirstName("Jose");
    father.setLastName("Pereira");

    Persontype mother = new Persontype();
    mother.setFirstName("Maria");
    mother.setLastName("Pereira");

    result.setFather(father);
    result.setMother(mother);


    Persontype olderSon = new Persontype();
    olderSon.setFirstName("John");
    olderSon.setLastName("Pereira");

    Persontype youngerSon = new Persontype();
    youngerSon.setFirstName("Ana");
    youngerSon.setLastName("Pereira");
    result.getChildren().add(olderSon);
    result.getChildren().add(youngerSon);

    return result;
}

So, my straight question is: based on my scenario described above with wsdl and its implementation, is the really only way to check if the return from web service is null by using isPresent() in a very similar way we were used to do with classical null checks (if (f != null){...)?

I think this is opinion based question here... you could have the `if-else` *as-well* as the `Optional.ofNullable`, it's just a matter of choice. The thing is I would go for the `if/else` and stream operations inside the `{}` block, it looks somehow cleaner for me — Eugene, Dec 14 '17 at 11:25
Calling `.filter(Objects::nonNull)` on an `Optional` doesn’t make any sense. The whole point of an `Optional` is that the encapsulated object can never be `null`. When the optional is empty, the filter predicate will never be evaluated. So the effect of `.filter(Objects::nonNull)` is the same as `.filter(x -> true)`, you get back the original optional in either case. — Holger, Dec 14 '17 at 11:34
I'd recommend modifying, if possible, the `helloWorldClientImplBean.allFamily()` to return `Optional` instead of a nullable `Family` reference. Instead of wrapping the result in `Optional.ofNullable` it'd be better to push the creation of `Optional` into the callee, so that the calling code that's shown here is more amenable to using `Optional` directly. This is preferable to forcing callers to either do null checks or do their own `Optional` wrapping. — Stuart Marks, Dec 14 '17 at 19:42
@Stuart Marks, thanks. Very usefull note. I will have this in mind when coding my real scenario. — Jim C, Dec 15 '17 at 08:46

score 6 · Accepted Answer · answered Dec 14 '17 at 11:52

The main misconception is to assume that there was a need to do operations like .filter(Objects::nonNull) on an optional. It would defeat the entire purpose of optionals, if such filtering was necessary for empty optionals. Especially, as the result of filtering is again an empty optional when the predicate evaluates to false, bringing you back to square one.

In fact, .filter(Objects::nonNull) has the same effect as .filter(x -> true), for non-empty optionals it’s always true, for empty optionals it will never get evaluated anyway.

Further, you are switching back to an if statement despite you already know about ifPresent. So one straight-forward solution derived from your original code would be

Optional.ofNullable(helloWorldClientImplBean.allFamily())
        .ifPresent(f -> f.getChildren().stream()
            .filter(x -> x.getFirstName().equalsIgnoreCase("John"))
            .findFirst()
            .ifPresent(y -> System.out.println(y.getLastName()));

You can reduce the nested part by changing the operation to

Optional.ofNullable(helloWorldClientImplBean.allFamily())
        .flatMap(f -> f.getChildren().stream()
            .filter(x -> x.getFirstName().equalsIgnoreCase("John"))
            .findFirst())
        .ifPresent(y -> System.out.println(y.getLastName()));

This only addresses the issue you described in your question, i.e. that the service allFamily() might return null. You have also included a new null check into the stream operation that would handle cases where child instances are null.

If that really is required, the best solution would be a kick into the butt of whoever is responsible for the service implementation, but anyway, the second best solution is to simply do

Optional.ofNullable(helloWorldClientImplBean.allFamily())
        .flatMap(f -> f.getChildren().stream()
            .filter(x -> x!=null && x.getFirstName().equalsIgnoreCase("John"))
            .findFirst())
        .ifPresent(y -> System.out.println(y.getLastName()));

which is simpler than inserting an additional .filter(Objects::nonNull) into the stream.

Plus one for "kick the butt of whoever is responsible for the service implementation." — Stuart Marks, Dec 14 '17 at 19:32
@Hoger, it seems you don't agree about . I understand that minOccurs=0 can result in null. Well, in my real current work, I see this a lot. In the government xsd, I see cases like and I understand it can be null as well. — Jim C, Dec 15 '17 at 09:01
A reasonable API should return an empty list instead of `null`. But that has been addressed in the first part of my answer already; the second part of my answer refers to your `null` check applied to the stream elements, which would be required only if the list is not `null`, not empty, but contains `null` elements. That’s the worse offender that led to my remark about the service implementor, *if* such a scenario is really possible. If not, the solutions of the first part of my answer are already sufficient. Though returning an empty collection instead of `null` still would be preferable. — Holger, Dec 15 '17 at 09:10
@Holger, I totaly agree but I can't guarantee who coded the API in government side will never return null (children = null in my simple example). The only documentation I have is the xsd which clear declares: minOccurs="0" so, technically saying, null is possible as far as I understand. — Jim C, Dec 15 '17 at 09:21
As said, the `children == null` case is handled in the first part and I wouldn’t start a war about it. However, the `children != null && children.contains(null)` case should never happen. — Holger, Dec 15 '17 at 09:24

score 2 · Answer 2 · answered Dec 14 '17 at 13:33

2

For handling the result with Optional, please refer to Holger's answer. Here I would like to take a different approach.

Make yourself one single question: why would you need Optional to handle this case at all? Is it to avoid using a null check with an if block on the returned Family value?

Consider this code:

Family f = helloWorldClientImplBean.allFamily();

if (f != null) {
    f.getChildren().stream()
        .filter(x -> x.getFirstName().equalsIgnoreCase("John"))
        .findFirst()
        .ifPresent(y -> System.out.println(y.getLastName()));
}

It is crystal-clear, easy to read and easy to maintain.

Now consider, i.e. a code like this:

Optional.ofNullable(helloWorldClientImplBean.allFamily())
    .map(Family::getChildren)
    .map(Collection::stream)
    .map(stream -> stream.filter(x -> "John".equalsIgnoreCase(x.getFirstName())))
    .flatMap(Stream::findFirst)
    .map(Persontype::getLastName)
    .ifPresent(System.out::println);

This code is functional style. All the operations are performed over the Optional via Optional.map operations, except for the one that returns the first element of the stream, which is accomplished via Optional.flatMap. The stream is handled step by step and not as a one-liner. Every operation performed over the Optional is null safe (by this I mean that I'm not only checking whether the returned initial Family instance is null, but also i.e. if f.getChildren() returns null as well).

Which version is shorter? Which one is more elegant? Which one clearer and easier to understand? Which one expresses the intention of the programmer in the best way?

I know my answer...

answered Dec 14 '17 at 13:33

fps

33,623
8
55
110

I have to be honest that Holger has answered completely my question but both he and you added a lot in my learning moment. However, since you asked me "why would you need Optional to handle this case at all?", I would like to return another question: my above case is very simple, but and if I have a more complex nested types? Now, I am coding a client that is going to call a government service and all data from a company will be retrieved in a single call. So, the hierarch tree is much more complex and I am trying to figure out if Optional is worth in this case – Jim C Dec 14 '17 at 14:13
You can have a scratch idea how complex is looking my other two questions: https://stackoverflow.com/questions/47283660/is-it-possible-to-use-twice-ifpresent-each-one-checking-different-return-or-bifu and https://stackoverflow.com/questions/47414987/how-effciently-read-soap-web-service-response-with-nested-objects-using-streams. Firstly, I need to understand well your and Holger answers and then try to move forward in my hello world example increasing the hierarch and I will be able to answer "Which one clearer and easier to understand?" – Jim C Dec 14 '17 at 14:16
1

Hi @JimC ! Good luck with that client for a government service (I had to do it several times, not a very nice task in my case)... Consuming services is hard, and there's no easy way to achieve nice, simple and easy-to-read code in your client, i.e. you will have to handle special cases, null checks, consistency checks, provide default values, etc. Just be sure to document every little assumption you make and every convention or definition about the returned values, etc. As to whether go for `if / for` vs `Optional / Stream`, I've found that there's no simple answer... – fps Dec 14 '17 at 14:25
1

I always try to keep my code as easy to understand as possible, not for me, but for other programmers that may need to maintain my code in the future. In this sense, I've found that using *only* `Optional` and `Stream` are good for simple cases, but for complex cases I've found that it's much better to use all the tools you have at your disposal, so I've mixed both `Optional / Stream` with traditional `if / for` blocks. This is debatable and I don't claim I have the only truth here. In my experience, though, it's always better to make your code express your actual intentions. – fps Dec 14 '17 at 14:29
2

@JimC Maybe you find [this video](https://www.youtube.com/watch?v=Ej0sss6cq14) from Stuart Marks (JDK developer) about `Optional` enlightening. It's 1 hour long, but it's worth every minute. – fps Dec 14 '17 at 14:32
@StuartMarks Hi! You're most welcome. You know, I used to chain the world with `Optional`, but finally understood (the hard way, what else?) that writing *cool* code that uses *the newest* features doesn't always lead to good results. Your talk helped me a lot to reach this conclusion. – fps Dec 14 '17 at 19:38

how use only higher-order functions to check if the return of web service is null using Optional and avoid noSuchElementException

2 Answers2