How to handle logging of null values in a Java Optional chain

Question

Say I'm using optionals, and I've got:

A a;
....
Optional.ofNullable(a)
    .map(A::getB)
    .map(B::getC)
    .ifPresent(c -> ...) // do something with c

However, if for whatever reason, I want to log when B::getC is null (or when a is null). Is there some idiom for dealing with this? I could do a bunch of nested ifPresent calls, but that is very verbose.

The best I could come up with was a FunctionalHelper class, which wraps some Optional methods and adds others to support logging (log4j):

private static final Logger LOG = //...
private static final FunctionalHelper FH 
    = new FunctionalHelper(LOG, Level.DEBUG);

FH.ofNullable(a, Level.WARN, "You gave me a null A"))
    .map(FH.logNull(A::getB, "b was null for a: {}", a-> FH.args(a.getName()))
    .map(B::getC)
    .ifPresent(c -> ...) // do something with c

It works, but feels a bit like a bolted-on solution. Is there some idiom (or at least a standard library) for dealing with this kind of thing?

(I'm also wondering how to cleanly deal with checked exceptions thrown within the chain, but perhaps that's a separate question.)

UPDATE: In response to @Ole V.V., Here's the example with Optionals (UPDATE2: I tweaked it a bit to match log4j varags supplier semantics, but with functions:

private static final FunctionalHelper FH  = new FunctionalHelper(LOG, Level.DEBUG);

void foo(A someA) {
    FH.ofNullable(someA, Level.WARN, "You gave me a null A")
            .map(FH.logNull(A::getB, "B was null for A: {}", A::getName))
            .map(FH.logNull(B::getC, "C was null for B: {}", B::getName))
            .ifPresent(c -> { /* do something with c */});
}

Here's an If-else implementation:

void foo2(A a) {
    if (a == null) LOG.debug("You gave me a null A");
    else {
        B b = a.getB();
        if (b == null) LOG.debug("B was null for a: {}", a::getName);
        else {
            String c = b.getC();
            if (c == null) LOG.debug("C was null for a: {}", b::getName);
            else { /* do something with c */ }
        }
    }
}

I'd say the former is easier to read. Also easier to modify an existing Optional chain to add something when needed.

Answer 1

Just as @Ole said, Optional is not meant for this:

Instead of creating an Optional, create a stream of a singleton (I just noticed the existence of Stream::of).

You lose the convenience of Optional::map executing only if the element is present, so you have filter nulls yourself:

A a;
Stream.of(a)
  .peek(e->{if(e == null){LOG.debug("You gave me a null A");}})
  .filter(e->e != null) //Now you have to handle null values yourself
  .map(A::getB)
  .peek(e->{if(e == null){LOG.debug("B was null for A "+a.getName());}})
  .filter(e->e != null)
  .map(B::getC)
  .peek(e->{if(e == null){LOG.debug("C was null for B"+a.getB().getName());}})
  .filter(e->e != null)
  .findFirst() // Optional from now on.

In case you start with an optional, you can do:

Optional<A> optA;
Stream.of(optA.orElse(null))

The transition of stream to optional is smooth, optional to stream is not.

Answer 2

You shouldn't use such construct for two reasons:

1. First, you shouldn't mix and match null and Optional. Optional is really the replacement of null, api-wise. When you return null, then you have a problem. A::getB should return an Optional<B>, not a null. Okay, sometimes you can get API from the outside world, and you don't have the hands on A::getB, so if that's the case, you just get to work like this, but another way is to sanitize your inputs.

2. Second, you shouldn't log null values. If you log null values, this means that you have issues similar to NullPointerExceptions, and that means that you don't get what you expect. So this basically means that you should sanitize your inputs! It's okay to have this kind of statements when you're developing, but in production, you should be able to treat differently when a.getB() returns null and when b.getC() returns null. These days, it's usually better (because it's also easier) to use a debugger than use those logs about which value is null.

Both issues are resolved by sanitizing your inputs. Sanitizing your inputs means that your should map your inputs that aren't yours to something that is yours. The usual answer here is that there is code duplication: the API code and your code. Well, yes: there is the API model and your model, but it's not duplicate code: one is sanitized and the other not!

But anyways, what you expect is entirely possible with a simple wrapper. It's similar to what you wrote, indeed (even though you haven't shown the code), but I guess you already found the best way to handle this:

static <T,U> Function<T, U> logIfReturnsNull(Function<? super T, ? extends U> function, String functionName) {
  return input -> {
    U result = function.apply(input);
    if (result == null) {
      log.debug("{} returned null for input {}", functionName, input);
    }
    return result;
  };
}

Then you use it like this:

Optional.ofNullable(a)
  .map(logIfReturnsNull(A::getB, "A::getB"))
  .map(logIfReturnsNull(B::getC, "B::getC"))
  .ifPresent(c -> ...)