#377 Sweeping out nulls in the Fan libraries

JohnDG Wed 22 Oct 2008

It's quite helpful to see the null annotations in the FanDoc. It also reveals what is, in my opinion, an over reliance on null values.

For example, take Email. The field to may be null (along with many other fields), but allowing this field to be null produces bulky code:

if (e.to != null) {
    e.to.each |Str addr| { ... }
}

Preventing this field from null simplifies code considerably:

e.to.each |Str addr| { ... }

Is there any interest in patches that eliminate the use of nulls from the Fan libraries???

helium Wed 22 Oct 2008

What about

e.to?.each |Str addr| { ... }

in the meantime?

brian Wed 22 Oct 2008

The email libraries aren't really a good example because once we apply nullable checks to constructors all that will go away. In fact I've got a couple places in the code where I cast to (Obj?) just to check for null until we the ctor stuff finished.

I think looking at the sys APIs is probably a better place to look. There are two patterns where we use null:

  1. Virtually all methods that parse something or look something up use the "checked pattern". By default they return non-null or throw an exception. But you can also pass in an optional checked flag as false to have the method return null. This pattern has turned about be extremely useful at keeping the API concise. Examples:
    Int.parse
    Version.parse
    Pod.find
    Type.find
    Slot.find
    Thread.findService
  2. By convention any invalid result is returned as null. In Java often times -1 is used, but I've stayed away from that pattern because null is actually easy to use (where originally would have been painful in Java) and also because virtually anything which takes an index allows negative numbers to index from the end. Examples:
    Str.index
    Int.toDigit
    InStream.read

Of course all these APIs were designed before we had nullables. But at the same time I do think null is the right choice in many cases.

katox Wed 22 Oct 2008

By default they return non-null or throw an exception. But you can also pass in an optional checked flag as false to have the method return null.

Is there a good reason why it should be a parameter? I don't think one would like to compute the exact behaviour at run time. I guess most of the time you just pass a constant to the call. If this is the case, the compiler could actually compute all the constant expressions and use it to infer the return type.

Int? f(Bool mustReturnFlag) {
    if (flag)
       // find or throw an exception
    else
       // find or return null
}

Int x := f(true)  // ok
Int? y := f(true) // ok, Int enhanced to Int?
z := f(true)      // ok, Int inferred within the pod, Int? outside the pod

But the behaviour outside the declaring pod would be less pleasing (and unfortunately the most common case for APIs):

Int x2 := f(false)  // compilation error
Int? y2 := f(false) // ok
z2 := f(false)      // ok, Int? inferred within and outside the pod

The same if the parameter was a computed value (not a constant):

Bool b := hardFunc();
Int x2 := f(b)  // compilation error
Int? y2 := f(b) // ok
z2 := f(b)      // ok, Int? inferred within and outside the pod

There could be a convention - paired methods with this behaviour. I know it clutters the interface (in a way) but the contract is simpler and can be fully enforced (and checked) by the compiler.

Besides that there is always a possibility to have a bug in the function implementation causing the caller NPE (by declaring the non-null return parameter in documentation but returning null anyway). If that happens one can't avoid defensive coding (which will turn to dead code once the issue is fixed).

Maybe the pairing could get a bit of syntactic sugar like

Int? find() {
    ...
}
Int find!() {
     ret := find()
     if (ret == null)
        throw Err()

     return ret  // ok, the compiler can prove that ret is non-null 
}

which would make it more bearable.

brian Thu 23 Oct 2008

Yes the obvious solution for the "checked" methods would be to use a pattern with two methods. If we can find the right naming convention, I might like that. The naming has to use a suffix though - because I want these methods to be right next to each other in the documentation. Although I don't personally find leaving them as null a big deal since by definition these methods are expected to fail one way or the other (raise exception or return null).

Login or Signup to reply.