Is it okay to use exceptions as tools to "catch" errors early?

Question

I use exceptions to catch problems early. For example:

public int getAverageAge(Person p1, Person p2){
    if(p1 == null || p2 == null)
        throw new IllegalArgumentException("One or more of input persons is null").
    return (p1.getAge() + p2.getAge()) / 2;
}

My program should never pass null in this function. I never intend it to. However as we all know, unintended stuff happens in programming.

Throwing an exception if this problem occurs, allows me to spot and fix it, before it causes more problems in other places in the program. The exception stops the program and tells me "bad stuff happened here, fix it". Instead of this null moving around the program causing problems elsewhere.

Now, you're right, in this case the null would simply cause a NullPointerException right away, so it might not be the best example.

But consider a method such as this one for example:

public void registerPerson(Person person){
    persons.add(person);
    notifyRegisterObservers(person); // sends the person object to all kinds of objects.
}

In this case, a null as the parameter would be passed around the program, and might cause errors much later, which will be hard to trace back to their origin.

Changing the function like so:

public void registerPerson(Person person){
    if(person == null) throw new IllegalArgumentException("Input person is null.");
    persons.add(person);
    notifyRegisterObservers(person); // sends the person object to all kinds of objects.
}

Allows me to spot the problem much before it causes weird errors in other places.

Also, a null reference as a parameter is only an example. It could be many kinds of problems, from invalid arguments to anything else. It's always better to spot them early.

---

So my question is simply: is this good practice? Is my use of exceptions as problem-preventing tools good? Is this a legitimate application of exceptions or is it problematic?

Answer 1

Yes, "fail early" is a very good principle, and this is simply one possible way of implementing it. And in methods that have to return a specific value, there isn't really very much else you can do to fail deliberately - it's either throwing exceptions, or triggering assertions. Exceptions are supposed to signal 'exceptional' conditions, and detecting a programming error certainly is exceptional.

Answer 2

Yes, throwing exceptions is a good idea. Throw them early, throw them often, throw them eagerly.

I know there's an "exceptions vs. assertions" debate, with some kinds of exceptional behavior (specifically, the ones thought to reflect programming errors) handled by assertions that can be "compiled out" for runtime rather than debugging/testing builds. But the amount of performance consumed in a few extra correctness checks is minimal on modern hardware, and any extra cost is far outweighed by the value of having correct, uncorrupted results. I've never actually met an application codebase for which I'd want (most) checks removed at runtime.

I'm tempted to say I wouldn't want a lot of extra checking and conditionals within the tight loops of numerically intensive code...but that's actually where a lot of numerical errors are generated, and if uncaught there, will propagate outward to effect all results. So even there, checks are worth doing. In fact, some of the best, most efficient numerical algorithms are based on error evaluation.

One final place to be very conscious of extra code is very latency-sensitive code, where extra conditionals can cause pipeline stalls. So, in the middle of operating system, DBMS, and other middleware kernels, and low-level communication/protocol handling. But again, those are some of the places errors are most likely to be observed, and their (security, correctness, and data integrity) effects to be most damaging.

One improvement I have found is to not throw only base-level exceptions. IllegalArgumentException is good, but it can come from essentially anywhere. It doesn't take much in most languages to add custom exceptions. For your person-handling module, say:

public class PersonArgumentException extends IllegalArgumentException {
    public MyException(String message) {
        super(message);
    }
}

Then when someone sees a PersonArgumentException, it's clear where it's coming from. There's a balancing act about how many custom exceptions you want to add, because you don't want to multiply entities unnecessarily (Occam's Razor). Often just a few custom exceptions are enough to signal "this module isn't getting the right data!" or "this module can't do what it's supposed to do!" in a way that is specific and tailored, but not so over-precise that you have to re-implement the entire exception hierarchy. I often come to that small set of custom exceptions by starting with stock exceptions, then scanning the code and realizing that "these N places are raising stock exceptions, but they boil down to the higher-level idea that they're not getting the data they need; let's replace those stock exceptions with a higher-level exception that more clearly communicates what's really going on."

Answer 3

Failing as soon as possible is great when debugging an application. I remember a particular segmentation fault in a legacy C++ program : the place where the bug was detected had nothing to do with the place where it was introduced (the null pointer was happily moved from one place to another in memory before it finally caused a problem). Stack traces cannot help you in those cases.

So yes, defensive programming is a really effective approach to detect and fix bugs quickly. On the other hand, it can be overdone, especially with null references.

In your specific case, for example: if any of the reference is null, the NullReferenceException will be thrown at the next statement, when trying to get the age of one person. You don't really need to check things by yourself here: let the underlying system catch those errors and throw exceptions, that's why they exist.

For more realistic example, you can use assert statements, which :

1. Are shorter to write and read:

       assert p1 : "p1 is null";
       assert p2 : "p2 is null";
   

2. Are specifically designed for your approach. In a world where you have both assertions and exceptions, you can distinguish them as follows:

   • Assertions are for programming errors ("/* this should never happen */"),
   • Exceptions are for corner cases (exceptional but probable situations)

Thus, exposing your assumptions about the inputs and/or state of your application with assertions let the next developper understand a little more the purpose of your code.

A static analyzer (e.g. the compiler) might be happier too.

Finally, assertions can be removed from the deployed application using a single switch. But generally speaking, don't expect to improve efficiency with that: assertions checks at runtime are negligible.

Answer 4

As far as I know different programmers prefer one solution or the other.

The first solution is usually preferred because it is more concise, in particular, you do not have to check the same condition again and again in different functions.

I find the second solution, e.g.

public void registerPerson(Person person){
    if(person == null) throw new IllegalArgumentException("Input person is null.");
    persons.add(person);
    notifyRegisterObservers(person); // sends the person object to all kinds of objects.
}

more solid, because

1. It catches the error as soon as possible, i.e. when registerPerson() is called, not when a null pointer exception is thrown somewhere down the call stack. Debugging becomes much easier: we all know how far an invalid value can travel through the code before it manifests itself as a bug.
2. It decreases coupling between functions: registerPerson() does not make any assumptions about which other functions will end up using the person argument and how they will use it: the decision that null is an error is taken and implemented locally.

So, especially if the code is rather complex, I tend to prefer this second approach.

Answer 5

In general, yes, it is a good idea to "fail early". However, in your specific example, the explicit IllegalArgumentException does not provide a significant improvement over a NullReferenceException - because both objects operated upon are delivered as arguments to the function already.

But lets look at a slightly different example.

class PersonCalculator {
    PersonCalculator(Person p) {
        if (p == null) throw new ArgumentNullException("p");
        _p = p;
    }

    void Calculate() {
        // Do something to p
    }
}

If there was no argument checking in the constructor, you would get a NullReferenceException when calling Calculate.

But the broken piece of code was neither the Calculate function, nor the consumer of the Calculate function. The broken piece of code was the code that tries to construct the PersonCalculator with a null Person - so that is where we want the exception to occur.

If we remove that explicit argument check, you will have to figure out why a NullReferenceException occurred when the Calculate was called. And tracking down why the object was constructed with a null person can get tricky, especially if the code constructing the calculator is not close to the code that actually calls the Calculate function.

Answer 6

Not in the examples you give.

As you say, throwing explicitly isn't gaining you much when you're going to get an exception shortly thereafter. Many will argue that having the explicit exception with a good message is better, though I don't agree. In pre-released scenarios, the stack trace is good enough. In post-release scenarios, the call site often can provide better messages than inside the function.

The second form is providing too much info to the function. That function shouldn't necessarily know that the other functions will throw on null input. Even if they do throw on null input now, it becomes very troublesome to refactor should that stop being the case since the null check is spread throughout the code.

But in general, you should throw early once you determine that something has gone wrong (while respecting DRY). These though are maybe not great examples of that.

Answer 7

In your example function, I would prefer you did no checks and just allow the NullReferenceException to happen.

For one, it makes no sense to be passing a null there anyway, so I'll figure out the problem immediately based on the throwing of a NullReferenceException.

Two, is that if every function is throwing slightly different exceptions based on what kind of obviously wrong inputs have been provided, then pretty soon you've got functions that might throw 18 different types of exceptions, and pretty soon you find yourself saying that's too much work to deal with, and just suppressing all exceptions anyway.

There's nothing you can really do to help the situation of a design-time error in your function, so just let the failure happen without changing it.