2

I was pondering about removing unnecessary null condition checks from my code today mostly involving deep nested objects. For example:

I have a class Big and it has a reference to Small object and its getters and setters, likewise Small has reference to Tiny object and its getters and setters and Tiny object has the tinyLength variable which is an Integer variable.

So, for example, if I'm writing a method that take in a Big object parameter and returns tinyLength at the very end, I usually do something like this:

public Integer getTinyLength (Big big) {
    if (big!=null && big.getSmall()!=null && big.getSmall().getTiny()!=null &&
        big.getSmall().getTiny().getTinyLength()!=null) {
    return big.getSmall().getTiny().getTinyLength();
}
else {
    // throw exception or return null or do whatever
}

So my question here is rather than doing all this gunk, why don't I just do something like:

try {
    return big.getSmall().getTiny().getTinyLength();
}    
catch (Exception e){
    //null pointer exception being a subclass of Exception class   
    log your error here 
    throw new NullPointerException();
}

I feel like the second code snippet is very concise and to the point. Is there any disadvantages to doing something like this? Especially when there are multiple nested objects, and dereferencing each one them usually results in highly unreadable code / also prone to mistakes if you miss one of them and it results in null.

AJNeufeld
  • 8,526
  • 1
  • 25
  • 44
shashwatZing
  • 1,550
  • 1
  • 17
  • 24
  • You may find this helpful http://stackoverflow.com/questions/14813877/advantages-and-disadvantages-of-using-try-catch – Anurag Joshi Sep 28 '16 at 17:01
  • This is addressed in Groovy and Kotlin gracefully. I wouldn't bother to deal with such long null checks in pure Java. – Alexiy Sep 28 '16 at 17:04
  • Well, You can surely go for it. It will make code much more readable. According to [this](http://stackoverflow.com/questions/16451777/is-it-expensive-to-use-try-catch-blocks-even-if-an-exception-is-never-thrown) question, using try-catch blog is not _that_ expensive. – Hardik Modha Sep 28 '16 at 17:12
  • 1
    how likely is it that any of those objects could be null? If its not likely at all (exceptional even) that is what exceptions are intended for. however I think if those objects are frequently null (by design) it is going to be more costly catching an exception than avoiding it entirely. If your app isn't performance critical I wouldn't worry about it. – trooper Sep 28 '16 at 17:19
  • 1
    I would try to catch the specific exception though. don't lump everything into `Exception` - that isn't good practice and will bite you eventually. – trooper Sep 28 '16 at 17:23

1 Answers1

1

The first code sample is inefficient, and thread unsafe. getSmall() is called four times, getTiny() is called three times, and getTinyLength() is called twice.

The second code sample is dangerous. Catching Exception when you just want to catch NullPointerException can lead to catching exceptions you didn't mean to catch. Plus, the overhead of constructing a NullPointerException object, throwing it, catching it, discarding it, and creating yet another NullPointerObject is non-trivial.

Better would be:

Integer length = null;
if (big != null) {
    Small small = big.getSmall();
    if (small != null) {
        Tiny tiny = small.getTiny();
        if (tiny != null) {
            length = tiny.getTinyLength();
        }
    }
}
if (length != null) {
    return length;
} else {
    // Throw exception, return null, or do whatever
}

Better still would be to hide that code away in an getter inside Big#getTinyLength().

A serious look at your data model should be in order. Can you have a Big that doesn't have a Small? Can you have a Small that doesn't contain a Tiny? Why would Tiny not have a length? If these members shouldn't ever be null, then let the JVM check for it and throw an exception, but then don't catch it, just to rethrow an equivalent one:

return big.getSmall().getTiny().getTinyLength();

EDIT

If you are using Java 8, then the Optional type could simplify your code:

public Integer getTinyLength (Big big) {
    return Optional.ofNullable(big)
        .map(Big::getSmall)
        .map(Small:getTiny)
        .map(Tiny::getTinyLength)
        .orElse(null);
}

This constructs an Optional<Big> from big, and proceeds to unwrap it to get to the tiny length value. If big is null, or if any mapping function used in the .map(...) calls returns null, an empty optional is produced. Finally, the value of the Optional<Integer> is returned if present, and null otherwise.

AJNeufeld
  • 8,526
  • 1
  • 25
  • 44