Hacker News Reader

Comment by bajsejohannes | Hacker News Reader

I've worked with a code base in C++ where the code base would accumulate significant amounts of

    if (argumentX == null)
        return null;

at the top of function signatures. It was just defensive programming. Maybe argumentX couldn't be null, but it would take time to figure that out (sometimes I did that, though). More code means harder to read and maintain, thus costing dollars.

This would also be contagious: If a piece of code checks if X is null, you'll assume that X can be null, whether or not that's true.

I'd certainly prefer to be able to reason about the code with the safe assumption that certain things cannot be null.

View on HN

I'd certainly prefer to be able to reason about the code with the safe assumption that certain things cannot be null.

You can do this, but if you want it to be maintainable, you'll also want to detail in the function comments this technical debt. If you don't, someone else will come along and see your sweet method (looking only at the comments) and use it where the input can be null.

Ex:

  /**
  * This does some stuff.
  * @param entry Does something with this
  * DEBT: Assumes the input entry is not null.
  */
  void doSomething(SomeObject entry) { }

Is that C++? Just use a reference if you want a pointer which asserts it can't be null.

It's Java ;P I haven't used C++ in a long time. But you are correct a reference does exactly what he wants.

For anyone who needs to look it up like me: http://en.wikipedia.org/wiki/Reference_(C%2B%2B)

Except the reference can itself be null, which is still problematic.

Why do you consider this a debt? It's pretty much the only sane thing to do.

If people want to pass lazy NullPointerExceptions, it's their fault.

Why just put it in the documentation? Documentation is liable to drift from implementation, and AFAIK no compiler or runtime verifies the accuracy of comments. I'd feel much better about adding asserts to the original code, leaving it for a few generations of testing and exposure, and then eventually remove the conditionals. The assert calls then function as executable documentation.

Depending on your user-base (i.e. if you distribute headers to other developers with precompiled code), the documentation may be necessary on its own... but it's much weaker than an assert.

Java has supported @Nullable and @Nonnull for a while, it's pretty standard in good Java code these days, and the IDE's will perform static analysis to make sure you use these annotations consistently (e.g. warn you if you are checking a @Nonnull against null or if you are forgetting to test a @Nullable against null before dereferencing it).

I've worked on a lot of legacy Java code where these kind of null checks cause problems. The end up confusing things in corner cases.

You should verify arguments at a top level, then let the code underneath blow up with a NullPointerException if something unexpected happened. Your stack trace will point at where the problem lies.

I find for some reason that a lot of people want to use null instead of empty lists where you'll end up with this ugliness:

    if (list != null) {
        for (Thing t: list) {
            processThing(t);
        }
    }

Instead of just passing in a Collections.emptyList() instead.

I agree with one exception: if you're storing the value passed down into a structure which will survive the call, then the value should be checked for null before being placed in the structure. This is because the eventual null pointer exception may occur long after it was put in the structure, obscuring how it got there.

Even if it can't be null now the calling code may change in the future. And did you check all the possible error conditions (e.g. failed allocation). If you are relying on a non-null call best to check every time.

Reply on news.ycombinator.com