I want use Optional of nullable this is code I have.

 private Appointment buildAppointmentDetail(UserDetail userDetail) {
    List<StopDetail> stopDetails = userDetail.getStopDetailList();
    return stopDetails != null
            ? stopDetails
            .stream()
            .min(Comparator.comparing(StopDetail::getStopNumber))
            .map(this::buildAppointment)
            .orElse(Appointment.builder()
                    .build())
            :null;
}

CodePudding user response：

Your code is a ternary operation:

1. If getStopDetailList() returns null, then this method also returns null.
2. If getStopDetailList() returns an empty list, return an empty appointment.
3. If getStopDetailList() returns anything else (i.e. a non-empty list), return a single appointment that captures that StopDetail on the list that has the lowest stopNumber, ignoring all other values in the list.

Generally, when you write code like this, think twice. null works best if it has no semantic meaning - in other words, most code that interacts with null ought to be crashing, because null by design ought to mean "This value is unset / not applicable / unknown", in other words, turn the fact that attempting to ask anything from a null value results in NPEs to your advantage.

Here's one trivial way that might help: Perhaps instead .getStopDetailList() should not be a ternary itself, and should be specced to only ever return a list. It might be empty. Right now your code acts as if there is a fundamental difference between an empty stop list and a null value, and 'propagates' this ternary state itself. Double check that this is in fact correct, and if it is, consider returning something else; generally returning null like this is a bad idea because null by definition cannot 'answer any question' (return a value from a method invocation) in any way other than throw new NullPointerException().

If truly that was your intent and you want this function to deal with the 'completely unknown state' (namely, null) by passing that right on through as your current snippet appears to, I suggest you write this:

 List<StopDetail> stopDetails = userDetail.getStopDetailList();
 if (stopDetails == null) return null;

 return stopDetails.stream()
            .min(Comparator.comparing(StopDetail::getStopNumber))
            .map(this::buildAppointment)
            .orElse(Appointment.emptyAppointment());

By exiting early, code becomes easier to read: In your code, the null handling 'bookends' your code: I look all the way at the top of the method to see the null check and then have to look all the way at the last line of the method to realize what happens when it is. There is no easy way to compress 'walk through the stop details, fetch the one with the smallest stop number, then build an appointment out of that, returning an empty appointment for empty lists' down to something that reads near instantly, but you CAN compress the 'if the stoplist is returned as null, just propagate that right out immediately' concept.

If you insist on using optional, you can, but it'd be silly and ugly - your code is longer, less efficient, and harder to read. It seems, as a consequence, utterly pointless. For completion's sake:

return Optional.ofNullable(userDetail.getStopDetailList())
  .map(list -> list.stream()
    .min(Comparator.comparing(StopDetail::getStopNumber))
    .map(this::buildAppointment)
    .orElse(Appointment.emptyAppointment())
  ).orElse(null);

This is a hopelessly complicated mess as it interweaves all sorts of cases and cannot be easily 'unweaved'. (You can't orElse first and then map).

Note that many styleguides require braces for all ifs, but nevertheless don't say anything of note about piling stream methods and using short form lambdas on and on. If you think about that for a few seconds you realize actually operating 'to the letter' is utterly farcical - that's just not a consistent view at all. The actual reason is that your styleguide is most likely out of date - not written to deal with the 'power' to write long and complex method invoke chains with lambdas, or to write lambdas themselves in the first place. That or it's just a bunch of rules somebody wrote down without keeping in mind a solid idea on what the style guide is supposed to accomplish.

In other words, ignore that. if (details == null) return null; all on line line, without braces, is fine. Better than fine - it's more readable than the alternative.