5

I'm exploring the possibility of eliminating null while refactoring part of a codebase I'm working on. We've upgraded to java 8, so we have Optional<T> at our disposal. In order to do this effectively, we need to make sure null isn't being passed to any of our methods (this is after we wrap any potential null values within Optional that enter our system from external services/libraries). The obvious way to handle this is to explicitly check for null and throw IllegalArgumentException if necessary, however, this would be unreasonably verbose and manual. Is there a less manual/more terse way of doing this?

w.brian
  • 16,296
  • 14
  • 69
  • 118

5 Answers5

10

You can use Objects::requireNonNull for exactly this. For example, in a constructor you can do:

this.myField = Objects.requireNonNull(myField);

It will throw a NullPointerException if you pass null.

Avoid the overuse of Optional. See for example this answer.

Community
  • 1
  • 1
Paul Boddington
  • 37,127
  • 10
  • 65
  • 116
  • What consistutes "overuse" of `Optional`? Are you saying there are scenarios when an object could be null, and `Optional` shouldn't be used? – w.brian Oct 13 '15 at 13:51
  • 2
    @w.brian, I think the linked answer explains "overuse" quite well; e.g. you shouldn't return `Optional` from methods nor should you usually use `Optional` method arguments. – Mick Mnemonic Oct 13 '15 at 14:04
4

You may try Lombok annotation processor which has @NonNull annotation. Annotating the parameter with NonNull will automatically generate the null-check during the compilation, so you will got either NullPointerException or IllegalArgumentException (whichever you like) at runtime.

Tagir Valeev
  • 97,161
  • 19
  • 222
  • 334
  • By default it will throw `NullPointerException`. How can I override to my own exception? – mwKART Mar 15 '17 at 09:14
  • @mwKART, I think you cannot use your own inspection, only either NPE or IAE (configured via `lombok.nonNull.exceptionType` key) – Tagir Valeev Mar 16 '17 at 09:39
  • Where can i set this? can you give a snippet. Is it inside the class where you have declared @NotNull – mwKART Mar 23 '17 at 09:15
  • Refer here: https://projectlombok.org/features/configuration you need to have a file `lombok.config` in classpath. – Karthik R Sep 11 '18 at 07:20
3

I would use the following approach which combines using the @Nonnull annotation (static analysis) with an explicit, fail-fast runtime check through Guava's Preconditions:

import javax.annotation.Nonnull;
import static com.google.common.base.Preconditions.checkNotNull;

public void doSomething(@Nonnull MyClass input) {
    checkNotNull(input);
    /* Do something */
}

The @Nonnull annotation enables a compliant IDE or a static analysis tool such as FindBugs to flag client code that clearly violates the contract. As an additional layer of defence, checkNotNull guarantees that the method exits early and loudly with a NullPointerException if for some reason a null input got passed in.

Mick Mnemonic
  • 7,808
  • 2
  • 26
  • 30
1

There will be other answers that will propose ways to eliminate the checking code. These ways tend to work as if by magic, and I am not particularly fond of magic. So, I will propose to you a way of not completely eliminating the verbosity and the manual typing, but cutting it down to half.

Use assertions.

An assertion in java looks like this:

assert n != null;

As you can see, it occupies a single line instead of two, and it does not require the manual coding of a throw statement, if you can be satisfied with the AssertionError exception that it throws. Generally, we do not catch exceptions that indicate bugs, so for the most part you should be fine, but if you must really have an IllegalArgumentException, then you can code your assertion statement as follows:

assert n != null : new IllegalArgumentException( "n cannot be null" );

What will happen here if n is null is that a new AssertionError exception will be thrown, and the "cause" of this exception will be your IllegalArgumentException. So, the IllegalArgumentException will appear in the stack trace, albeit after the "Caused by:" line.

As an added benefit, once you have a system that passes all the tests and you know it works, you can refrain from supplying the -enableassertions (-ea for short) VM option, and your program will run slightly faster, since the assertions will not be evaluated.

Mike Nakis
  • 56,297
  • 11
  • 110
  • 142
  • 3
    `if (n == null) throw new IllegalArgumentException("n cannot be null");` will do it too. But the fact, that you can disable these assertions by VM option changes my point of view. So finally: great idea! – bobbel Oct 12 '15 at 22:07
  • 2
    @bobbel if you want to read more about the awesomeness of assertions, here is a post on my blog: http://blog.michael.gr/2014/09/assertions-and-testing.html – Mike Nakis Oct 12 '15 at 22:21
  • thanks for these great thoughts about assertions! Very interesting! – bobbel Oct 12 '15 at 22:46
  • 6
    The very fact that assertions can be (and by default are) disabled in a production environment implies that you _should not_ use them for parameter-checking; the inputs received by a live system are probably out of your control. Writing validation logic such as this behind assertions means that this potentially critical part of the program may not be executed in a production system. – Mick Mnemonic Oct 13 '15 at 00:55
  • @MickMnemonic let's not turn this into an "assertions are good" vs "assertions are bad" argument. If you are under the impression that there is any benefit whatsoever in keeping assertions enabled on production runs, then keep supplying the `-ea` option, and *require* that assertions are enabled in order to run. There is a very simple sequence of code that you can add to your `main()` which checks whether assertions are enabled. See my blog post for the code: http://blog.michael.gr/2014/09/assertions-and-testing.html – Mike Nakis Oct 13 '15 at 06:57
  • 4
    Assertions should *not* be abused to do parameter checking. That’s out of any discussion. Besides that, there is no point in turning off a `null` check. A `null` check imposes no significant overhead and is optimized away anyway if the caller never passes `null`. – Holger Oct 13 '15 at 08:22
  • 2
    @MikeNakis, perhaps you missed my point. Assertions are good, but only when used for the right purpose -- for checking conditions that the programmer thinks should _never_ happen (as explained nicely in [this answer](http://stackoverflow.com/a/1957662/905488)). Once a system "passes all the tests", you only know it works with the given test data; you shouldn't assert that your tests model a production system perfectly. For parameter-checking, using an `@Nonnull` annotation (for documentation and static analysis), supplemented with a runtime check for `null`, is a good combination. – Mick Mnemonic Oct 13 '15 at 09:09
  • So, really, there are two people who have downvoted because they think that "this answer is not useful". Gaaaa. – Mike Nakis Oct 13 '15 at 11:21
  • 2
    I did not downvote, but your answer definitely suggests a bad practice. Asserts are not for preconditions, but for establishing the invariants. Asserts are good inside the private logic of your program to document the current state of algorithm, etc. But preconditions must be checked in another way (in Java-7 and later preferably with `Objects.requireNotNull()`). – Tagir Valeev Oct 14 '15 at 02:25
  • That's some pretty nice piece of dogma you all have there. So, fine, you state your dogma, I state mine: Asserts are for everything but checked exceptions. (Or the kinds of exceptions that would be checked if we were still using checked exceptions.) – Mike Nakis Oct 14 '15 at 07:00
  • I'm not going to downvote, but I find your claim that not enabling asserts will make your code run faster to be dubious and should have a citation. The JIT compiler will assume that `n` is not null and will compile the null check out entirely (regardless of whether you use an `assert` or an `if () throw` until it actually finds `n` to be null a certain number of times. The `Objects.requireNonNull` call will also probably get inlined (https://stackoverflow.com/questions/10073025) and be a non-event. So, unless your code regularly uses that null check, the performance will be the exact same. – squid314 Feb 08 '17 at 23:23
  • @squid314 The jit compiler does not work by magic; it cannot "find n to be null a certain number of times" unless it checks for null each time. Anyhow, when I speak of code running faster I am not talking about assertions that check against null, I am talking about assertions that do a lot more. As an extreme example, take the practice of preceding a binary search with an assertion that checks that the array is actually sorted before the search. – Mike Nakis Feb 08 '17 at 23:41
  • @MikeNakis, the JIT will optimistically delete null checks entirely if they never trigger (see `l33tify` in https://a.blmq.us/2k7Bb8h, forgive the formatting) and will know what to do if that assumption is wrong. (I mis-remembered the number of failures.) Recognizing that you use assertions for much more than just these null checks, you might gain significant performance by removing them (or you might not since the JIT might try to delete the checks anyway), but the answer alone suggests that removing the null check will improve performance, which it probably won't and is misleading. – squid314 Feb 09 '17 at 17:20
  • @squid314 Thanks for the link, it was a very informative article, I learned some things I did not know. However, the conclusion of the article is: "The best way to help your compiler is to not try so hard to help it — just write your code as you otherwise would." --that's what I do. And my code naturally has lots of assertions. – Mike Nakis Feb 09 '17 at 18:33
0

The Checker framework provides an annotation @NonNull for allowing compile-time checking of whether code paths exist that could pass null values to an annotated reference.

See: https://checkerframework.org/api/org/checkerframework/checker/nullness/qual/NonNull.html

mernst
  • 7,437
  • 30
  • 45
Tripp Kinetics
  • 5,178
  • 2
  • 23
  • 37