0

I'm trying to refactor a piece of code that should handle a null checks the method is:

public static boolean isAnyNullArgs(Supplier<Object>... objs) {
    return Stream.of(objs).anyMatch(o -> o.get() == null);
}

and I call it like that:

if (!isAnyNullArgs(() -> value.getField1(),
                () -> value.getField1().getField2(),
                () -> value.getField1().getField2().getFiled3(),
                ...
)

is there any way I can refactor my method so that I can call it like this (with only 1 lambda):

if (!isAnyNullArgs(() -> value.getField1(),
                ,value.getField1().getField2(),
                ,value.getField1().getField2().getField3(),
                ...
)
Stefan Zobel
  • 3,182
  • 7
  • 28
  • 38
uri go
  • 105
  • 5

2 Answers2

4

It doesn't look like this can be done, since value.getField1().getField2() should only be evaluated if value.getField1() is not null. Otherwise you'll get a NullPointerException.

This suggests that your isAnyNullArgs() method is not very helpful.

You might as well write a single condition:

if (value.getField1() != null &&
    value.getField1().getField2() != null &&
    value.getField1().getField2().getField3() != null) {

}

Or, you can use Optionals instead:

if (Optional.ofNullable(value.getField1())
            .map(f1 -> f1.getField2())
            .map(f2 -> f2.getField3())
            .isPresent ())

This can be made more readable if you replace the lambda expressions with method references:

if (Optional.ofNullable(value.getField1())
            .map(Field1Class::getField2)
            .map(Field2Class::getField3)
            .isPresent ())
Eran
  • 387,369
  • 54
  • 702
  • 768
  • Optional is indeed the way to go in order to simulate (with reasonable code) a null-collapsing operator in Java like C# has: `val?.getField1()?.getField2()?.getField3() != null` would be the C# equivalent to OP's code and your Optional version. – Javier Martín Dec 09 '19 at 12:30
0

I wouldn't bother trying to abstract that. Prefer to get rid of the nulls. But I guess you could do something like:

@SafeVarargs
public static boolean isAnyNullArgs(Supplier<? extends Collection<?>>... objs) {
    return Stream.of(objs)
       .flatMap(os -> os.get().stream())
       .anyMatch(o -> o == null);
}

...

if (!isAnyNullArgs(() -> List.of(
    value.getField1(),
    value.getField1().getField2(),
    value.getField1().getField2().getFiled3(),
)) {

@Eran points out that in the OP example, if value.getField1() returns null then it will NPE. Likewise for the next line. Which just goes to show that you don't want to be throwing nulls about.

Tom Hawtin - tackline
  • 145,806
  • 30
  • 211
  • 305
  • @Eran That is true. That's nulls for you. – Tom Hawtin - tackline Dec 09 '19 at 12:33
  • But in this case you would get the NPE instead of just a true/false result, since you are actually calling all the functions before getting to isAnyNullArgs. Thus, if any method in the chain except for the last returns null, you get the exception that you were trying to avoid. – Javier Martín Dec 09 '19 at 12:36
  • To correct my previous comment, actually, the NullPointerException will be thrown inside isAnyNullArgs(), since you are passing it a lambda expression. – Eran Dec 09 '19 at 12:40