Search code examples
javaerror-handlingcallback

Java - Use null comparison or instanceof?


Below is a Java class constructor which accepts a callback for error handling. null should be an acceptable value, and if the passed value is null then skip error handling.

I have two ways of checking if the callback passed is able to be accepted, and my question here is which method (if either) would be the best for long-term maintainability and sanity. I am open to suggestions as well if anyone believes that there are better ways of going about this, as I am relatively new to using Java.

class ErrorMaker {
    public ErrorMaker(Consumer<Throwable> onError) {
        ClassLoader classloader = Thread.currentThread().getContextClassLoader();
        try (InputStream inputStream = classloader.getResourceAsStream("\\\\..\\..")) {
            Properties config = new Properties();
            config.load(inputStream);
            // ...
        } catch (Exception err) {

            // METHOD #1 - null comparison
            if (onError != null) {
                onError.accept(err);
            }

            // METHOD #2 - instanceof
            if (onError instanceof Consumer<?>) {
                onError.accept(err);
            }

        }
    }
}

Thanks in advance.


Solution

  • tl;dr

    Use Objects class for easier reading.

    if ( Objects.nonNull( onError ) ) {…}  // Replaces `if ( onError != null )`. 
    

    Simple & obvious > clever

    As the Comments explained, both of your solutions work technically:

    • if (onError != null) {…}
    • if (onError instanceof Consumer<?>) {…}

    Of the two, the first is preferable because it is obvious.

    The second appears to be testing for one of several possible subtypes. But you are making ever-so-clever use of the fact that instanceof also acts as a test for being non-null. Such clever code is to be avoided, IMHO, because it fails the most critical of tests: Will this code be crystal clear to the exhausted reader’s blurry eyes at 3 AM during an all-night emergency debugging session?

    Objects.nonNull( onError )

    Personally, I would recommend a third option: the convenience methods on Objects class, isNull, nonNull, and requireNonNull. English words (“nonNull”) are usually easier to read than mathematical signs (“!=“).

    if ( Objects.nonNull( onError ) ) {…}  // Replaces `if ( onError != null )`. 
    

    If null were not an acceptable/expected value, call Objects.requireNonNull. This method throws an exception upon encountering a null.

    Objects.requireNonNull( x ) ;  // Throws exception if `x` is null.