How better refactor chain of methods that can retu

Question

I have code like:

obj1 = SomeObject.method1();
if (obj1 != null) {
  obj2 = obj1.method2();
  if (obj2 != null) {
     obj3 = obj2.method3();
     if (obj3 != null) {
              ............


     return objN.methodM();

   }
  }
 }
....

I have near 10 steps. It seems very fragile and error prone. Is there a better way to check on null chained methods?

Thanks.

Answer 1

If you want are so trusting of your source objects that you plan to chain six of them together, then just continue to trust them and catch exceptions when they are thrown - hopefully rarely.

However if you decide not to trust your source objects, then you have two choices: add compulsive "!= null" checks everywhere and don't chain their methods...

OR go back and change your source object classes and add better null handling at the root. You can do it by hand (with null checks inside setters, for example), or alternatively you can use the Optional class in Java 8 (or Optional in Google's Guava if you're not on Java 8), which offers an opinionated null-handling design pattern to help you react to unwanted nulls the moment they are introduced, rather than wait for some poor consumer to encounter them later on.

Answer 2

I thinh this kind of question was already answered here. Especially see the second aswer about Null Object Pattern .

Answer 3

For getter method who has no parameter , try this:

Util.isNull(person, "getDetails().iterator().next().getName().getFullName()")

for most of the cases, it works well. Basically, it is trying to use java reflection to do the null check layer by layer, till it reach the last getter method , since we do a lot cacheing for the reflection, the code works well in production. Please check the code below.

public static boolean isNull(Object obj, String methods) {
    if (Util.isNull(obj)) {
        return true;
    }
    if (methods == null || methods.isEmpty()) {
        return false;
    }
    String[] A = methods.split("\\.");
    List<String> list = new ArrayList<String>();
    for (String str : A) {
        list.add(str.substring(0, str.indexOf("(")).trim());
    }
    return isNullReflect(obj, list);
}
public static boolean isNullReflect(Object obj, List<String> methods) {
    if (Util.isNull(obj)) {
        return true;
    }
    if (methods.size() == 0) {
        return obj == null;
    }
    Class<?> className = Util.getClass(obj);
    try {
        Method method = Util.getMethod(className.getName(), methods.remove(0), null, className);
        method.setAccessible(true);
        if (method.getName().equals("next")
                && !Util.isNull(Util.getMethod(className.getName(), "hasNext", null, className))) {
            if (!((Iterator<?>) (obj)).hasNext()) {
                return true;
            }
        }
        try {
            return isNullReflect(method.invoke(obj), methods);
        } catch (IllegalAccessException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        } catch (IllegalArgumentException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        } catch (InvocationTargetException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    } catch (SecurityException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }
    return false;
}


public static Boolean isNull(Object object) {
    return null == object;
}

public static Method getMethod(String className, String methodName, Class<?>[] classArray, Class<?> classObj) {
    // long a = System.nanoTime();
    StringBuilder sb = new StringBuilder();
    sb.append(className);
    sb.append(methodName);
    if (classArray != null) {
        for (Class<?> name : classArray) {
            sb.append(name.getName());
        }
    }
    String methodKey = sb.toString();
    Method result = null;
    if (methodMap.containsKey(methodKey)) {
        return methodMap.get(methodKey);
    } else {
        try {
            if (classArray != null && classArray.length > 0) {
                result = classObj.getMethod(methodName, classArray);
            } else {
                result = classObj.getMethod(methodName);
            }
            methodMap.put(methodKey, result);
        } catch (NoSuchMethodException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        } catch (SecurityException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }
    // long b = System.nanoTime();
    // counter += (b - a);
    return result;
}
    private static Map<String, Method> methodMap = new ConcurrentHashMap<String, Method>();

Answer 4

It's common problem for null references in java.

I prefer chaining with &&:

if (obj1 != null && obj1.method1() != null && obj1.method1().method2() != null)

Answer 5

You can chain them and surround everything with a try/catch and catch the NPE.

Like this:

try
{
    Object result = SomeObject.method1().method2().methodN();
    return result;
}
catch(NullPointerException ex)
{
     // Do the errorhandling here.
}

Outside that I second @Neil's comment: Try to avoid that kind of chains in the first place.

EDIT:

Votes show that this is very disputed. I want to make sure it is understood, that I do not actually recommend this!

Proceeding like this has many sideeffects and should be generally avoided. I just threw it into discussion for OPs special situation, only as one way to achieve the goal if not otherwise possible!

If anyone feels he needs to do this: Please read the comments for possible pitfalls!

Answer 6

Write like

obj1 = SomeObject.method1();
if (obj1 == null) 
    return;
 obj2 = obj1.method2();
 if (obj2 == null) 
    return;

etc. As a C developer this is a very common paradigm and is extremely common. If it's not possible to convert your code to this flat flow then your code needs to be refactored in the first place, regardless of what language it exists in.

Replace return with whatever you are actually doing in the case where these fail, whether that be return null, throw an exception, etc. - you've omitted that part of your code but it should be the same logic.