Is it acceptable to use exceptions instead of verb

2020-02-22 03:30发布

I recenly encountered this problem in a project: There's a chain of nested objects, e.g.: class A contains an instance variable of class B, which in turns has an instance variable of class C, ..., until we have a node in the tree of class Z.

     -----      -----      -----      -----               ----- 
     | A | ---> | B | ---> | C | ---> | D | ---> ... ---> | Z |
     -----      -----      -----      -----               -----  

Each class provides getters and setters for its members. The parent A instance is created by an XML parser, and it is legal for any object in the chain to be null.

Now imagine that at a certain point in the application, we have a reference to an A instance, and only if it contains a Z object, we must invoke a method on it. Using regular checks, we get this code:

    A parentObject;

    if(parentObject.getB() != null &&
        parentObject.getB().getC() != null &&
        parentObject.getB().getC().getD() != null &&
        parentObject.getB().getC().getD().getE() != null &&
        ...
        parentObject.getB().getC().getD().getE().get...getZ() != null){
            parentObject.getB().getC().getD().getE().get...getZ().doSomething();
    }

I know that exceptions should not be used for ordinary control flow, but instead of the previous code, I have seen some programmers doing this:

    try {
        parentObject.getB().getC().getD().getE().get...getZ().doSomething();
    } catch (NullPointerException e){}

The problem with this code is that it may be confuse when maintaining it, since it doesn't show clearly which objects are allowed to be null. But on the other hand is much more concise and less "telescopic".

Is it an acceptable to do this to save development time? How could the API be redesigned to avoid this problem?

The only thing I can think of to avoid the long null checking is to provide void instances of the nested objects and providing isValid methods for each one of them, but wouldn't this create a lot of innecesary objects in memory?

(I've used Java code, but the same question can apply to C# properties)

Thanks.

9条回答
Explosion°爆炸
2楼-- · 2020-02-22 04:14

I think you could provide static isValid methods on each class, for example for class A that would be:

public class A {
...
  public static boolean isValid (A obj) {
    return obj != null && B.isValid(obj.getB());
  }
...
}

And so on. Then you would have:

A parentObject;

if (A.isValid(parentObject)) {
  // whatever
}

However, although I won't get into you business I must say that such a method chaining does not say anything good about the design; maybe it's a sign of need for refactoring.

查看更多
趁早两清
3楼-- · 2020-02-22 04:15

Using exceptions seem a poor fit here. What if one of the getters contained non-trivial logic, and threw a NullPointerException? Your code would swallow that exception without intending to. On a related note, your code samples exhibit different behaviour if parentObject is null.

Also, there really is no need to "telescope":

public Z findZ(A a) {
    if (a == null) return null;
    B b = a.getB();
    if (b == null) return null;
    C c = b.getC();
    if (c == null) return null;
    D d = c.getD();
    if (d == null) return null;
    return d.getZ();
}
查看更多
淡お忘
4楼-- · 2020-02-22 04:29

Personally I like to avoid this problem altogether by using an option type. By adjusting the value returned from these methods/properties to be Option<T> rather than T the caller can choose how they wish to handle the case of no value.

An option type can either have a contained value or not (but the option itself can never be null), but the caller cannot simply pass it on without unwrapping the value so it forces the caller to deal with the fact there may be no value.

E.g. in C#:

class A {
    Option<B> B { get { return this.optB; } }
}

class B {
    Option<C> C { get { return this.optC; } }
}

// and so on

If the caller wants to throw, they merely retrieve the value without explicitly checking to see if there is one:

A a = GetOne();
D d = a.Value.B.Value.C.Value.D.Value; // Value() will throw if there is no value

If the caller wants to just default if any step doesn't have a value, they can perform mapping/binding/projection:

A a = GetOne();
D d = a.Convert(a => a.B) // gives the value or empty Option<B>
       .Convert(b => b.C) // gives value or empty Option<C>
       .Convert(c => c.D) // gives value or empty Option<D>
       .ValueOrDefault(new D("No value")); // get a default if anything was empty 

If the caller wants to default at each stage, they can:

A a = GetOne();
D d = a.ValueOrDefault(defaultA)
     .B.ValueOrDefault(defaultB)
     .C.ValueOrDefault(defaultC)
     .D.ValueOrDefault(defaultD);

Option is not currently part of C# but I imagine one day will be. You can get an implementation by referencing the F# libraries or you may be able to find an implementation on the web. If you'd like mine, let me know and I'll send it to you.

查看更多
登录 后发表回答