24

This might look like a primitive question or a this could be done by a simple utility library method that I don't know about.

The goal is to check the value of a boolean field that is nested under two objects.

private boolean sourceWebsite(Registration registration) {
    Application application = registration.getApplication();
    if (application == null) {
        return true;
    }

    Metadata metadata = application.getMetadata();
    if (metadata == null) {
        return true;
    }

    Boolean source = metadata.getSource();
    if (source == null) {
        return true;
    }

    return !source;
}

I know this could be done in a single if(). I have added multiple ifs here for the sake of readability.

Is there a way that we could simplify the above if statements and have a simple utility class that returns the value of Boolean source if the parent objects or not null?

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
codeMan
  • 5,730
  • 3
  • 27
  • 51

4 Answers4

39

You can use java.util.Optional in this way:

private boolean sourceWebsite(Registration registration) {
    return Optional.of(registration)
        .map(Registration::getApplication)
        .map(Application::getMetadata)
        .map(Metadata::getSource)
        .map(source -> !source)
        .orElse(Boolean.TRUE);
}

In short, this will return true if any of the getters returns null, and !Metadata.source otherwise.

Captain Man
  • 6,997
  • 6
  • 48
  • 74
ernest_k
  • 44,416
  • 5
  • 53
  • 99
15

The following will return true if any one of is null. If all values are not null, it returns !source.

private boolean sourceWebsite(Registration registration) {
      return registration.getApplication() == null 
      ||     registration.getApplication().getMetadata() == null
      ||     registration.getApplication().getMetadata().getSource() == null
      ||    !registration.getApplication().getMetadata().getSource();

}

Updated :

If you want that every getter not called more than once then you can declare variable for every object like 

private boolean sourceWebsite(Registration registration) {
      Application application;
      Metadata metadata;
      Boolean source;
      return (application = registration.getApplication()) == null 
      ||     (metadata = application.getMetadata()) == null
      ||     (source = metadata.getSource()) == null
      ||    !source;

 }
Khalid Shah
  • 3,132
  • 3
  • 20
  • 39
  • 5
    The null-conditional operator (?.) of c# is for shure a nice thing... – keuleJ Apr 11 '19 at 10:52
  • 3
    One problem I see is with that pattern is that it executes getApplication 4 times, getMetadata 3 times and getSource 2 times. If they are all trivial getters, this might not be that much of a problem. But if their implementation is non-trivial (or even worse: not side-effect free) this might become a problem. – Philipp Apr 11 '19 at 11:45
  • @Philipp yes. But in general getters are trivial. – Khalid Shah Apr 11 '19 at 11:47
  • 5
    In general they *should* be side-effect free, but that assumes that you are working with classes written by people who knew what they were doing and followed good software engineering principles and Java best practices. In the real world, that's a very brave assumption. – Philipp Apr 11 '19 at 11:49
  • @Philipp yeah thats a brave assupmtion :D . Thanks for giving a deeper look :) – Khalid Shah Apr 11 '19 at 11:51
  • If any one of *what*? – Peter Mortensen Apr 11 '19 at 12:15
  • @PeterMortensen any one of the object which he wants to check. – Khalid Shah Apr 11 '19 at 12:16
  • @Philipp: Wouldn't the code be optimized so that `getApplication` gets called once? – Eric Duminil Apr 11 '19 at 15:01
  • 1
    @EricDuminil You generally can not rely on that. The JVM *might* be able to optimize these method calls away when they are trivial getters, but it might not be able to do that when they have some logic and it certainly won't be able to do it when they have side-effects. – Philipp Apr 11 '19 at 15:25
  • @EricDuminil see optimisation in updated answer. – Khalid Shah Apr 11 '19 at 15:55
  • I don't think this really answers OP's question, they specifically said "I know this could be done in a single `if()`. I have added multiple `if`s here for the sake of readability." – Captain Man Apr 11 '19 at 16:03
  • @CaptainMan isn’t readable? And it contain if () ? – Khalid Shah Apr 11 '19 at 16:04
  • @KhalidShah To me OP is saying they know they could do it in a single expression with chained `||`s like you've done. That said, after thinking about it a little more, I still think this answer is helpful because other people may not realize this approach even though it seems OP may have been aware. – Captain Man Apr 11 '19 at 16:08
2

Another option you can use is a try-catch block. If you get a null pointer exception return true.

private boolean sourceWebsite(Registration registration) {
    try {
        return !registration.getApplication().getMetadata().getSource();
    }
    catch (NullPointerException e) {
        return true;
    }
}
CaptianObvious
  • 163
  • 1
  • 4
  • 6
    You shouldn't use exceptions for program logic. Exceptions should be used in case of an unrecoverable state in a program where it's better to stop the entire flow rather than try and recover. – Nzall Apr 11 '19 at 13:46
  • 1
    In general I do agree with you (doing so might make the code harder to follow if it wasn't this short), and I feel that using Optional is correct choice here (assuming you are using a new enough version of java to have access to it). The question asked for a way to handle these checks without several null checks. This will do that. – CaptianObvious Apr 11 '19 at 13:53
  • 1
    @Nzall Exceptions can also be useful in program logic, for instance as a very readable way of checking if a string parses into a number which is very similar to CaptainObvious answer. – Old Nick Apr 11 '19 at 15:00
  • Backing up the comment from Nzall: https://stackoverflow.com/a/8255933/6296561 - TL;DR: exceptions are heavy – Zoe Apr 11 '19 at 15:08
  • 1
    Until a NPE is thrown much deeper in one of those methods. – Koekje Apr 11 '19 at 15:56
  • How can you be certain the NPE was thrown by one of your getters returning null then being used versus one of your getters throwing one internally for "real" reasons? Maybe there's a Map inside that wasn't properly set. You could easily hide an exception you didn't mean to. – Captain Man Apr 11 '19 at 15:57
  • See [this question](https://stackoverflow.com/q/9758457/1858327) for why it is bad. – Captain Man Apr 11 '19 at 16:15
-2

You could do it using a hacky method like this:

public static Object get(Object o, String... m) throws NoSuchMethodException, IllegalAccessException, InvocationTargetException {
    if (o == null || m.length == 0) {
        return null;
    }
    for (String m1 : m) {
        o = o.getClass().getMethod(m1).invoke(o);
        if (o == null) {
            return null;
        }
    }
    return o;
}

And call it like this:

Boolean source = (Boolean) get(registration, "getApplication", "getMetadata", "getSource");
return source == null ? false : !source;

But I wouldn't do it this way in any serious projects.

Mika Lammi
  • 1,278
  • 9
  • 21
  • 2
    This is bad code in so many aspects: slow, not controlled by the compiler, immune to refactoring aids from the IDE. Even you recognize that you wouldn't use this in any serious projects. But then: what's the point? No one would bother to post a question or even read the answers for unserious projects. – julodnik Apr 11 '19 at 14:56
  • It could become a bit better if you passed Methods as parameters instead of strings. It would probably look a bit like `return Optional.of(registration).map(Registration::getApplication).map(Application::getMetadata).map(Metadata::getSource)` so you might as well use `Optional` directly. Still, I don't think you deserved downvotes. This method is still interesting as a thought experiment. – Eric Duminil Apr 11 '19 at 16:38