Null pointers everywhere because data is suddenly sparse.

Question

Someone designed code that relied on full data; the XML always had every element. The data source is now sending sparse XML; if it would have been empty before, it's missing now. So, it's time to refactor while fixing bugs.

There's 100+ lines of code like this:

functionDoSomething(foo, bar, getRoot().getChild("1").getChild("A").
    getChild("oo").getContent());

Except now, getChild("A") might return null. Or any of the getChild(xxx) methods might.

As one additional twist, instead of getChild(), there are actually four separate methods, which can only occur in certain orders. Someone suggested a varargs call, which isn't a bad idea, but won't work as cleanly as I might like.

What's the quickest way to clean this one up? The best? "try/catch" around every line was suggested, but man, that's ugly. Breaking out the third argument to the above method into it's own function could work... but that would entail 100+ new methods, which feels ugly, albeit less so.

The number of getChild(xxx) calls is somewhere between six and ten per line, with no fixed depth. There's also no possible way to get a correct DTD for this; things will be added later without a prior heads up, and where I'd prefer a warning in the logs when that happens, extra lines in the XML need to be handled gracefully.

Ideas?

getChild() is a convenience method, actually. The cleanest way I have in mind is to have the convenience methods return a valid Child object, but have that "empty" Child's getContent() always return "".

Answer 1

The solution is to use a DTD file for XML. It validates your XML file so getChild("A") won't return null when A is mandatory.

Answer 2

If it always drills down to roughly the same same level, you can probably refactor the code using Eclipse, for example, and it'll automatically change every line that looks the same.

That way you can modify the method to be smarter, rather than modifying each line individually

Answer 3

What you described (returning a special child object) is a form of the NullObject pattern, which is probably the best solution here.

Answer 4

How about:

private Content getChildContent(Node root, String... path) {
    Node target = root;
    for ( String pathElement : path ) {
         Node child = target.getChild(pathElement);
         if ( child == null ) 
            return null; // or whatever you should do

         target = child;
    }

    return target.getContent();

}

to be used as

functionDoSomething(foo, bar, getChildContent(root, "1", "A", "oo"));

Answer 5

Please consider using XPATH instead of this mess.

Answer 6

Your problem could be a design problem: Law of Demeter.

If not you can use something like an Option type changing the return type of getChild to Option<Node>:

for(Node r : getRoot())
  for(Node c1 : r.getChild("1"))
    for(Node c2: c1.getChild("A"))
      return c2.getChild("oo")

This works because Option implements Iterable it will abort when a return value is not defined. This is similary to Scala where it can be expressed in a single for expression.

One additional advantage is that you can define interfaces that will never return a null value. With an Option type you can state in the interface definition that the return value may be undefined and the client can decide how to handle this.

---