requireNotNull vs sure operator !! in Kotlin

6.9k views Asked by At

We have a Kotlin based application and recently we added third party code quality tools (Detekt in Codacy). However, we started facing UnsafeCallOnNullableType errors. We found that approach viable to use is to add requireNotNull checks on all parameter which maybe null. Currently, we are using sure operator (!!)

Do we have any specific reason or convention to chose one over the other. As far as I see, both will throw an Exception and will block the execution flow, except one will throw IllegalArgumentException while the other will throw NullPointerException.

3

There are 3 answers

1
Zoe is on strike On BEST ANSWER

requireNotNull, assuming you're referring to Objects#requireNonNull, is a Java method that is equivalent to !!, but with a different exception.

You didn't add any code, so it's slightly hard to help you debug. You mentioned third party code quality tools, but not which. I randomly stumbled over this GH issue that matches the error you're having. It's also the only thing I can find that would at any point use that exact error. I might have missed some, but it's the one that covers the top Google hits, so I'm going to go off that.

If you are using Detekt, this is a reported bug. Using !! is even suggested by IntelliJ.

However, you can do it another way.

Yes, using Objects#requireNonNull is one option. There is a second one though, and that's using the null-safe operator, as m0skit0 mentioned.

The reason this works is because if anything called is null, the final result is null. I.e. this:

instance.nonNullType.nullable?.nullableChild?.someOtherNullableChild

If any of the nullable ones are null, the final result is null, and none of the others are called.

Now, considering this is likely a bug in Detect, this seems like the easiest workaround for now:

whatever.calls.you?.make?.to?.the?.database ?: throw NullPointerException("Something is null");

It also keeps the variable non-null, which means you don't need null-safe calls later. The elvis operator checks if anything is null, then throws an exception. Alternatively, you can just use Objects#requireNotNull:

Objects.requireNonNull(whatever.calls.you.make.to.the.database)

If you really need to validate every single step, you'll just need to keep null checks everywhere

TL;DR:

!! and requireNotNull are effectively identical in how they work, except requireNotNull is a method call and !! compiles to an if-statement:

if(whatever == null) {
    Intrinsics.throwNpe();
}

The reason !! triggers UnsafeCallOnNullableType is because of a (probable) bug in Detekt. Both of the options are syntactic sugar for the same thing, though: if the variable is null, throw an NPE.

2
m0skit0 On

In my experience the best practice when a value is nullable is to use ? and ?: operators and provide an alternative (if available) when the value is null, for example:

settings?.getValue("some-setting") ?: defaultValue

Note that this expression will return defaultValue when either settings or getValue return null.

It is best to try to avoid using !! operator at all since this basically defeats any protection against nulls. When this is not possible, I would throw a more comprehensive exception instead of relying on generic exceptions like IAE or NPE, for example:

settings?.getValue("some-setting") ?: throw SettingNotFound("Descriptive message")
0
Ed Ballot On

As you mentioned, requireNotNull() throws IllegalArgumentException and !! throws NullPointerException. That could be helpful if you want to differentiate between defensive code added by your devs versus undefended code (by using !! is isn't immediately obvious).

However, the bigger benefit of using requireNotNull() is using the function that takes the lazyMessage argument. This way your devs can add more meaningful messages to the exception, which could help in debugging