0

I am having an issue with returning null value if condition is false with this function:

private <Optional>ServiceElements getPrice(final ServiceRequest serviceRequest,
                                  final Date date,
                                  final Integer code) {
if (isNotAFilter(serviceRequest.getCustomerId(),
    serviceRequest.getStaffId()) {
  return custRepository.getCustData(date, code).stream().filter(e -> e.getPrice() != null).filter(e -> e.getPrice >=0)
    .findFirst();  
}
return null; 
}

private double calculate(final ServiceRequest serviceRequest) 
{
return getPrice(serviceRequest).get().getPrice(); 
}

A java.lang.NullPointerException: null was thrown instead of returning null value when calling getPrice method.

Cael
  • 556
  • 1
  • 10
  • 36
  • Does this answer your question? [What is a NullPointerException, and how do I fix it?](https://stackoverflow.com/questions/218384/what-is-a-nullpointerexception-and-how-do-i-fix-it) – kaya3 Nov 19 '20 at 11:17

2 Answers2

0
if (isNotAFilter(serviceRequest.getCustomerId(),
    serviceRequest.getStaffId()) {
  price = custRepository.getCustData(date, code);
}

If the above if condition evaluated to false, your variable price is null. Then your return statement throws a NullPointerException.

Steephen
  • 14,645
  • 7
  • 40
  • 47
  • adding an else statement and setting price to null will then result in code smell...Any suggestion? – Cael Nov 19 '20 at 06:49
  • Your variable `price` is initialized as `null`, so no need of an `else` statement to set it to `null` – Steephen Nov 19 '20 at 06:52
  • how do i make it so that it will return null if the condition is false instead of throwing an Exception that will cause error? – Cael Nov 19 '20 at 11:01
0

Why bother with the temporary price variable?

private ServiceElements getPrice(final ServiceRequest serviceRequest,
                                      final Date date,
                                      final Integer code) {
if (isNotAFilter(serviceRequest.getCustomerId(),
    serviceRequest.getStaffId()) {
  return custRepository.getCustData(date, code).stream().filter(e -> e.getPrice() != null).filter(e -> e.getPrice >=0)
    .findFirst().orElse(null);
}
return null 
}

By the way, as you are already using Java's functional constructs then IMO it would make more sense like that:

private Optional<ServiceElements> getPrice(final ServiceRequest serviceRequest,
                                      final Date date,
                                      final Integer code) {
    if (isNotAFilter(serviceRequest.getCustomerId(),
        serviceRequest.getStaffId()) {
      return custRepository.getCustData(date, code).stream().filter(e -> e.getPrice() != null).filter(e -> e.getPrice >=0)
        .findFirst();
    }
    return Optional.empty(); 
}

This means you can more easily pipe this function with other functions without the ugly null checks everywhere. Also .orElse(null) is a more complex way to just .get().

Tarmo
  • 3,851
  • 2
  • 24
  • 41
  • thanks for the help. however when i call the getPrice method from calculate method (edited post), an exception was thrown instead of returning null value when the condition is false. Any idea why? – Cael Nov 19 '20 at 15:54
  • Optionals all the way dude :) `getPrice(serviceRequest).map(ServiceElements::getPrice)` would return another Optional. Point is to avoid `get()`. But at some point your going to have the face the fact that the price does not exist because the condition was false. So either somewhere call `if (priceOptional.isEmpty()) ... do something ...` or provide default value like that: `getPrice(serviceRequest).map(ServiceElements::getPrice).orElse(0.0)` (not sure if 0.0 is the correct default value, this depends on your business logic). – Tarmo Nov 20 '20 at 07:02