Pragmatic way of handling nested null checks in existing xmlbeans Java codebase?

Question

This is a common enough question, but I need to say that the question you'll suggest this is a duplicate of doesn't contain solutions that are appropriate to our situation.

At work, we're using xmlbeans on an 8 year old codebase and to put it delicately it's not been handled well. Of course the real solution is that we shouldn't use null as information (and probably not be using xmlbeans at all), but I'm looking at the least terrible way of handling the following problem:

If we have an entity of Person, which can optionally have subentity of Wallet, which can optionally have subentities of WalletItems, which can optionally have subentity of Card which can have a CardNumber, what's the most practical way of checking whether a Person has at least one CardNumber?

We've got three working solutions, as well as an argument over which one the team should stick to:

isCardHolder(Person person){
    if (person != null && person.getWallet != null && 
        person.getWallet.getWalletItems != null &&
        person.getWallet.getWalletItems.get(0) != null && // Just a dirty example, don't worry here
        person.getWallet.getWalletItems.get(0).getCard != null &&
        person.getWallet.getWalletItems.get(0).getCard.getCardNumber != null){
        return true;
    } else {
        return false;
}

Or:

isCardHolder(Person person){
    Wallet wallet = (Person != null ? person.getWallet : null);
    WalletItems[] walletItems = (wallet != null ? wallet.getWalletItems : null);
    // etc etc
    if (card.getCardNumber != null) {
        return true;
    } else {
        return false;
    }
}

Or even:

isCardHolder(Person person){
    try {
        if (person.getWallet.getWalletItems.get(0).getCard.getCardNumber != null){
            return true;
        }
    } finally {
        return false;
    }
}

My personal preference is to burn it down and start over, or to at least have move away from xmlbeans so we could use Null Object patterns or anything else in the standard question on this topic but we're stuck with it for now, too much depends on it.

score 1 · Accepted Answer · edited Jun 20 '20 at 09:12

Some quotes by Robert C. Martin "Clean Code" about functions.

The first rule of functions is that they should be small.

Functions should do something, or answer something, but not both.

IMHO the best way to do this could be something like:

    package com.example.sampl;
    
    public class MainSampl {
    
    
    
        public static void main(String[] args) {
            Person p = new Person();
            boolean correct = isCorrect(p);
            System.out.println(correct);
        }
    
        private static boolean isCorrect(Person person){
            return  isNotNull(person) &&
                    isNotNull(person.getWallet()) &&
                    isNotNull(person.getWallet().getWalletItems()) &&
                        isNotNull(person.getWallet().getWalletItems().getItems().get(0));
//etc
        }
    
        private static boolean isNotNull(Object data) {
            return data != null;
        }
    
    }

So I think that my isNotNull function is answering if parameter is fine and then isCorrect function is answering that all preferable to check objects are ok or not. Your functions:

Checking if objects are null's and answering if this is ok or not. So do both things
Second is doing the same as first one and in my opinion it is very unreadable for me. So not readable and do both things.
Third the same. It is doing two things answering and do something and it is too long.

And from me about using logical operators. I think that if you have less logical operators in your code it is better for future refactoring and better for reading it in general because

if you want to go fast, if you want to get done quickly, if you want your code to be easy to write, make it easy to read

Pragmatic way of handling nested null checks in existing xmlbeans Java codebase?

1 Answers1