Is it acceptable to use exceptions instead of verb

问题:

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.

回答1:

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.

回答2:

It is bad design if parentObject needs to know that A contains a B which contains a C wich contains.... That way, everything is coupled to everything. You should have a look at the law of demeter: http://en.wikipedia.org/wiki/Law_Of_Demeter

parentObject should only call methods on its instance variable B. So, B should provide a method that allows for the decision, e.g.

public class A {
  private B myB;
  //...
  public boolean isItValidToDoSomething(){
    if(myB!=null){
      return myB.isItValidToDoSomething();
    }else{
      return false;
    }
  }
}

Eventually, at the level of Z, the method has to return true.

Imho, saving development time is never a reason for tolerating problems in the design. Sooner or later these problems will steal you more time than it would have taken to fix the problems in the first place

回答3:

It's bad practice to use Exceptions here.

There's a hint in the name: Exceptions are for exceptional circumstances (i.e. unexpected) . If nulls are expected values, then encountering them is not exceptional.

Instead, I'd have a look at the class hierarchy and try to understand why such deep access chaining needs to happen. This seems like a big design issue, you shouldn't normally expect the caller to construct calls using deep knowledge of the structure of objects hidden within class A.

Questions you could ask:

• Why does the caller need to doSomething() with the Z object anyway? Why not put the doSomething() on class A? This could propagate doSomething() down the chain if needed and if the relevant field was not null....
• What does a null mean if it exists in this chain? The meaning of a null will suggest what business logic should be employed to handle it.... which could be different at each level.

Overall, I suspect the right answer is to put doSomething() on each level of the heirarchy and have the implementation something like:

class A {
  ...
  public void doSomething() {
    B b=getB();
    if (b!=null) {
      b.doSomething();
    } else {
      // do default action in case of null B value
    }
  }
}

If you do this, then the API user only has to call a.doSomething(), and you have the added bonus that you can specify different default actions for a null value at each level.

回答4:

Well, it depends on exactly what you're doing in the catch. In the above case, it appears that you want to call doSomething() if it's available, but if it isn't you don't care. In this case I would say that trapping the specific exception you're after is just as acceptable as a verbose check to ensure you won't throw one to begin with. There are many "null-safe" methods and extensions that use try-catch in a very similar manner to what you propose; "ValueOrDefault"-type methods are very powerful wrappers for exactly what's been done with the try-catch, for exactly the reason try-catch was used.

Try/catch is, by definition, a program flow control statement. Therefore, it is expected to be used to "control ordinary program flow"; I think the distinction you are trying to make is that it should not be used to control the "happy path" of normal error-free logic flow. Even then I might disagree; there are methods in the .NET Framework and in third-party libraries that either return the desired result or throw an exception. An "exception" is not an "error" until you cannot continue because of it; if there's something else you can try or some default case the situation can boil down to, it can be considered "normal" to receive an exception. So, catch-handle-continue is a perfectly valid use of try-catch, and many uses of exception throwing in the Framework expect you to handle them robustly.

What you want to avoid is using try/catch as a "goto", by throwing exceptions that aren't really exceptions in order to "jump" to the catch statement once some condition is satisfied. This is definitely a hack, and thus bad programming.

回答5:

The problem with the "catch an exception" approach is that it seems a bit heavy-handed. The exception stack trace should show you where it failed since your method names make it quite clear where you are in the hierarchy but it is not a good way of going about it. Plus how would you recover from the exception and carry on to a good state of your code?

If you must keep this very deep hierarchy then you could use static instances of each object which defines an "empty" state. The best example I can think of which does this is the C# string class which has a static string.Empty field. Then each call of getB(), getC() ... getZ() would return either a real value or the "empty" state, allowing you to chain the method calls.

By making the "empty" state instances static there would only be one of each type in your system. But you would need to consider what an "empty" state looks like for each type in your hierarchy and make sure it doesn't affect any other part of your application inadvertently.

回答6:

In Python, they encourage the style of "easier to ask forgiveness than permission", which could be applied here to say that it's better to just optimistically try to get to Z without safety checking, and let the exception handler fix a miss. That's easier to code, and it's more performant if the call of Z not being in the call chain is less likely than the case that it will be.

Aside from violating a bunch of OOP good design principles and exposing deeply nested private members, this code also seems vaguely dynamic in nature. That is, you want to call method X but only if X exists on the object, and you want that logic to apply to all objects in a hierarchy of unknown length. And you can't change the design because this is what your XML translation gives you.

Can you change languages then? Statically-typed C# may not be the best choice for what you're doing here. Maybe using Iron Python or some other language that's a little looser on typing will let you more easily manipulate your DOM. Once you've got the data in a stable state, you can pass that off to C# for the rest.

回答7:

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();
}

回答8:

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.

回答9:

I agree with the other answers that this should not need to be done, but if you must here is an option:

You could create an enumerator method once such as:

    public IEnumerable<type> GetSubProperties(ClassA A)
    {
        yield return A;
        yield return A.B;
        yield return A.B.C;
        ...
        yield return A.B.C...Z;
    }

And then use it like:

    var subProperties = GetSubProperties(parentObject);
    if(SubProperties.All(p => p != null))
    {
       SubProperties.Last().DoSomething();
    }

The enumerator will be lazily evaluated leading to no exceptions.