CODEWITHSUNDEEP
javajava-8java-streamoption-type
gabru101
gabru101

Reputation: 65

Use Java8 map over Optional.Empty()

I have a piece of code where am doing something like:

Optional<College> college = Optional.ofNullable(student)
        .map(stud ->  stud.getCollege())
        .get()
        .stream()
        .filter(college -> Objects.nonNull(college.getCollegeName()))
        .findFirst();

Now, while writing an unit test, I got a catch that what if student comes as null?

It would be effectively like:

Optional.empty()                         // the same as the student is null
        .map(stud -> stud.getCollege())
        .get()
        .stream()
        .filter(college -> Objects.nonNull(college.getCollegeName()))
        .findFirst();

Which I think is not fine because I am getting Exception

expected<com.src.exceptions.CollegeNotFoundException> but
was<java.util.NoSuchElementException>

#Update

Updating the question details for clarifications

  1. Yes stud.getCollege() returns a list<>

Upvotes: 1

Views: 9586

Answers (2)

Nikolas
Nikolas

Reputation: 44496

Calling Optional::get with no previous check Optional::isPresent is dangerous because it might produce CollegeNotFoundException. And it is not the way the Optional shall be used. The idea of Optional is mapping/filtering the values and providing a default value if the Optional ends up with no element (empty).

Assuming Student::getCollege returns List<College> having method College::getCollegeName, you can do the following:

College college = Optional.ofNullable(student)
    .map(stud -> stud.getCollege())
     // if Optional is empty, then use an empty collection
    .orElse(Collections.emptyList())                              
    .stream()
    .filter(c -> Objects.nonNull(c.getCollegeName())) 
    .findFirst()
     // get the value or else college is null 
    .orElse(null);

As long as stud.getCollege() returns null, the Optional becomes empty and an empty list will be streamed. And again the same principle is applied: As long as the list is empty, the filter and findFirst are not be called and null is safely returned (or any default value you wish).

Also note that the line .filter(c -> Objects.nonNull(c.getCollegeName())) might also produce NullPointerException as long as there is not guaranteed stud.getCollege() doesn't return a list with a null element (remember the list is not null itself so Optional treats it as a "valuable" item). The safe code actually looks like:

Optional<College> college = Optional.ofNullable(student)
    .map(stud -> stud.getCollege())
    .orElse(Collections.emptyList())
    .stream()
    .filter(c -> c != null && c.getCollegeName() != null)
    .findFirst();

Actually, I prefer to return either a null-object, null or Optional itself.

Upvotes: 1

Tashkhisi
Tashkhisi

Reputation: 2262

I agree with @Nikolas approach except that you should not return null, returning null at last is against using Optional What about this one:

Optional<College> optional = Optional.ofNullable(student)
                .map(stud -> stud.getCollegeList())
                .orElse(Collections.emptyList())
                .stream()
                .filter(c -> Objects.nonNull(c.getCollegeName()))
                .findFirst();

Upvotes: 3

Related Questions