Saturday, April 4, 2009

Exceptional APIs

Axioms for public API design:

Axiom 1
NullReferenceExceptions that come out of your code ARE YOUR FAULT!

Axiom 2
The fewer exceptions your code can possibly throw, the better!

Applications of these axioms:

Let's say we're writing a public API, you and I. In it, let's say we have the following awesome method:
public void Foo(string s) {
    Bar(s.Length);
}
You will notice that we dereference s to get its Length property. If some misguided user of ours were to pass null to Foo, an NRE would pop out of our code like an overweight stripper out of a wedding cake. Awkward! And then the stack trace would get passed all over school and all the kids would laugh at us.

Lemma 1
From axiom 1 it follows: Null-check EVERYTHING YOU DEREFERENCE unless you know where it came from.

We could do a conditional dereference:
public void Foo(string s) {
    if (s != null) Bar(s.Length);
}
Or we could verify the argument:
public void Foo(string s) {
    if (s == null) throw new ArgumentNullException("s");
    Bar(s.Length);
}
If our method absolutely needs to dereference the object in order to do its job, then argument verification is the way to go. This may seem like trading one exception type for another, but it's really not. Argument exception types are perfectly acceptable - they inform our misguided user what went wrong and how to make it right. On the other hand, NREs mean that we didn't verify the argument or null-check before dereferencing. And they mean that all the kids will laugh at us.

Let's say that we decide on argument verification. Now let's say that we want to add a convenience overload:
public void Foo() {
    Foo("");
}
Looks good, right? WRONG!

Lemma 2
From axiom 2 it follows: Use 'null' as a sentinel between overloads for the default value when null is not an otherwise permissible value.

My favorite non-fiction book of all time, Framework Design Guidelines, specifically says not to do this. It is wrong. Well, sort of. FDG advises against using null as a "magic" sentinel, period. I argue that sentinel null is correct for parameters which are omitted in convenience overloads. Here is why:

To recap, our API currently looks like this:
public void Foo() {
   Foo("");
}

public void Foo(string s) {
    if (s == null) throw new ArgumentNullException("s");
    Bar(s.Length);
}
Let's consider the possible ways our misguided user can use this API. If he or she calls the convenience overload, or passes a string literal, everything's honkey dorey:
Foo(); // This is fine
Foo("How the hell do I use this API?"); // This too is just fine

But if misguided user passes null to Foo(string), or passes a variable which may be null, things aren't so rosey:
Foo(null); // Exception city!
Foo(someString); // It depends...

How is our misguided user to know what is and is not a safe call? They could look at our documentation. Assuming we wrote any. And assuming we correctly documented all of the parameters which need to be non-null. Or they could test passing null and see if it throws.

(As a side note, I wonder all the time about whether I can pass null to an API. Documentation is usually no help and if I see a parameterless overload, I generally assume that I can)

More likely, they will do none of the above and just write something resembling the last example call, passing a variable. A variable which is usually not null. A variable which is never null during testing. But a variable which, when released into the wild may, under some unforeseen circumstance, be null. Then all the kids would cry.

The solution:
public void Foo() {
    Foo(null);
}

public void Foo(string s) {
    if (s == null) s = "";
    Bar(s.Length);
}
All possible inputs to this API are valid and there is zero change of an exception. This is better!

Some may complain about semantics purity or somesuch. They are wrong.

So, if you have some public API which takes a parameter that a) is not allowed to be null, and b) has a default value for the purpose of convenience overloads, use the sentinel null. Just do it.