Quick poll when you know for sure that something i...

# announcements

[Czar]

Quick poll when you know for sure that something is not null, while it appears nullable, which construct should you use?
1️⃣ checkNotNull(something)
2️⃣ something!!
If you have an explanation for your preference I encourage you to post it as a thread comment.
Example:

// hibernate is used for illustration, because it recommends to use nullable IDs for entities
val entity: MyEntity = hibernateRepository.requireEntityById("some-id")
private fun String.doSomethingWithId()
// 1
checkNotNull(entity.id).doSomethingWithId()
// 2
entity.id!!.doSomethingWithId()

[louiscad]

checkNotNull if it's an argument so it throws an IllegalArgumentException

[gildor]

No

[gildor]

require throws IllegalArgumentException

[gildor]

check throws IllegalStateException

[louiscad]

Yes, you're right

[louiscad]

requireNotNull I meant

[louiscad]

Both take an optional lambda used for the exception message

[Czar]

Currently I use 1️⃣ , my justification is that !! is almost invisible when reading code and while for cases like this one it is probably ok, it makes a habit of substituting checkNotNull with !! in other situations, too, which isn't desirable.
By the way I've updated the code, because 1) I made a mistake when writing it first time. 2) to make it obvious that repository either returns a non-null entity or throws.

[gildor]

checkNotNull semantically more correct because throws IllegalStateException, which is correct exception in this case

[Czar]

The problem is I still can't get rid of a feeling that I'm doing unnecessary boilerplate 😞

[miha-x64]

Nullable IDs are evil. If class reflects a database entity, it must have an ID.

[Czar]

I agree. I'll probably have to refactor this at some point. It's a 3M+ LOC codebase though, split into several projects and libraries, that are developed by 5 geographically separated teams of developers 😐 And on top of that it is actually in fact 10 different products based off common code base...

[tipsy]

If you're sure, use double bang

[tipsy]

Isn't that the point of double bang?

[Czar]

@tipsy I did explain above why I don't like the idea of double bang. You're suggesting to use it without addressing the stated concerns.

[tipsy]

i only saw this as your agument against it:

it makes a habit of substituting checkNotNull with !! in other situations, too, which isn't desirable.

but that's very subjective. i always use !! when i'm sure, and but never otherwise.

!! is a statement; you're saying "this thing is never null". by using checkNotNull you're saying "this thing might be null", which is wrong.

[Czar]

by using checkNotNull you're saying "this thing might be null", which is wrong.

Nope, IllegalStateException is indicative of a bug, if state is illegal then it should never be achieved. So checkNotNull also states that the thing is never null. Note that never is not a real thing in software, it actually means never in correctly working code that is correctly accessed

[Czar]

By the way, quoting "Effective Java":

For example, this [IllegalStateException] would be the exception to throw if the caller attempted to use some object before it had been properly initialized.

[tipsy]

if i open a project and read `checkNotNull(myId)`/`requireNonNull(myId)/etc` i will (mistakenly?) think the id might be null. i will start wondering why and start investigating the id
if i read myId!!, i see that as a promise from whoever wrote the code that the id will "never" be null

i don't really care if the code throws IllegalStateException or NullPointerException. to me, !! more clearly shows the intent of whoever wrote the code

[tipsy]

it's all subjective ¯\_(ツ)_/¯

[tipsy]

when do you use !!, if not for these cases?

[tipsy]

https://discuss.kotlinlang.org/t/double-bang-vs-checknotnull/7952/2

[Czar]

Well, I do not use !!, I do not like !!, I think it was a mistake to include double bang in Kotlin language 🙂

[tipsy]

😄

[tipsy]

fair enough

[Czar]

I want to address this again:

if i open a project and read

`checkNotNull(myId)`/`requireNonNull(myId)/etc` i will (mistakenly?) think the id might be null

checkNotNull does not signify possibility of something being null
according to both IllegalStateException and checkNotNull documentation, it signifies that if at this point the argument is null then there's something wrong in the application, it doesn't work like it was intended and it is a bug which should be fixed.
Whereas !! is more like, here, compiler, I promise it's not null, leave me alone, I want to forget all about it. And this wish is granted, because when reading code, !! is almost invisible 🙂

[tipsy]

checkNotNull does not signify possibility of something being null

turns out it doesn't, but i didn't know that (neither did "darksnake" in the linked thread). "checkNotNull" reads like "this thing might be null". if you have 5 teams working on this codebase i suspect some of your devs might think like me and darksnake.

[aarjav]

I would think checkNotNull is used because there is some state in the current context that needs to be checked for null. If it breaks, it implies bug in current class or caller. !! means one of the methods or classes being used by current code returns a non null. If it breaks, it implies bug in method used (requireEntityById). Hopefully with Kotlin contracts we will be able to have contracts on val properties of arguments as well though I'm not sure since sometimes they're calculated. Imo !! should be used as little as possible and should be such that eventually a contract or some smarter typing can make it obsolete in the future.

[Czar]

@tipsy you have a point there, but I think that possible lack of knowledge in some of the members of the team should not influence code design, I find that code reviews and mentoring from senior members of a team take care of such finer points of coding as use of checkNotNull and IllegalStateException
@aarjav I mostly agree, although I'm a bit more strict and currently don't allow !! at all for my team.