Reputation: 1347
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.
Upvotes: 17
Views: 7811
Reputation: 28238
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
!!
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.
Upvotes: 8
Reputation: 3485
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
Upvotes: 3
Reputation: 25873
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")
Upvotes: 1