Kotlin: Don’t just use LET for null check

With the introduction of null safety in Kotlin, everybody now know this special standard function let{...}.

The example given in null safety document is as below

val listWithNulls: List<String?> = listOf(“Kotlin”, null)
for (item in listWithNulls) {
item?.let { println(it) } // prints Kotlin and ignores null
}

Hence let is inevitably being understood that it is the replacement of null check e.g. if (variable != null). Illustrated further below

// Conventional approach
if (variable != null) { /*Do something*/ }
// Seemingly Kotlin approach
variable?.let { /*Do something*/ }

While, this is permissible, it is not right to use let{...} all the time.

When not to use LET

Recommendation 1.1: When just checking null for an immutable variable, don’t use LET.

Imagine if you have a function that receive a nullable String as below. It may seems seems nice to do something as below.

// NOT RECOMMENDED
fun process(str: String?) {
str?.let { /*Do something*/ }
}

But if you check the decompiled Java code, it is

public final void process(@Nullable String str) {
if (str != null) {
boolean var4 = false;
/*Do something*/
}
}

It introduce a variable. If you process function is used lots of time, it is doing something extra without any benefits.

Instead just do the below

// RECOMMENDED
fun process(str: String?) {
if (str != null) {
// Do Something
}
}

The usage of str within the if scope is auto-cast to non-nullable already. The decompile code is more straightforward without extra variable introduced.

public final void process(@Nullable String str) {
if (str != null) {
/*Do something*/
}
}

Recommendation 1.2: If you just want to access the checked variable’s content, but not the outer scope class’ content, don’t use LET.

The reason you check for a variable is null or not, most likely you want to access it’s content.

Assume you have a webview and want to set it setting’s values if the setting is not null. The below seems reasonable

// NOT RECOMMENDED
webviewSetting?.let {
it.javaScriptEnabled = true
it.databaseEnabled = true
}

Instead just do the below

// RECOMMENDED
webviewSetting?.run {
javaScriptEnabled = true
databaseEnabled = true
}

Use run, as it would sent the variable as this to the scope, and this eliminate the need of it

Recommendation 1.3: If you need to chain the original variable content after the scope, don’t use LET.

Assuming you have a nullable list of String, where you want to just print the the size of it, and then chain it to do something else (e.g. for simplicity, assuming want to print each of the String).

One way to do it as below

Note: For simplicity purpose, assume for some reason I don’t want the chain to be in the inner scope of let with it.forEach{ println(it) }.
// NOT RECOMMENDED
stringList?.let {
println("Total Count: ${it.size}")
it
}
?.forEach{ println(it) }

This is not that nice, as we have to have the it there for the purpose of getting it return

Instead consider the below, using also

// THIS IS BETTER THAN PREVIOUS
stringList?.also {
println("Total Count: ${it.size}")
}?.forEach{ println(it) }

This would eliminate the need of explicit setting it

Note: The above better approach is not ideal (that’s why I didn’t write RECOMMENDED), as there’s multiple ?. I’ll illustrate more below why this is not ideal. But for simplicity reason I provide the example.

So clearly let is not the best solution in some case (even though they could achieve what’s intended logically).

Note: For better consideration of other scope function that let, you could check the below article

When to use LET

Recommendation 2.1: If you are checking for the null status of a mutable variable, use LET to ensure checked value is immutable.

This is the case where the state/content of the variable under checked could changed even after one has check it using if condition.

An example is a global variable as shown below

// RECOMMENDED
private var str: String? = null

fun process() {
str?.let { /*Do something*/ }
}

This make sense to use let (assuming the Do Something is meeting the recommendation 2.2 below)

The reason is because if we use normal if check, the smart casting can’t be apply to the variable check, due to its mutability. E.g.

// NOT RECOMMENDED
private var str: String? = null

fun process() {
if (str != null) {
println(str?.length)
}
}

The above show clearly, even within the if (str != null) scope, we can’t guarantee that str will not be null. Hence the ? is still needed if we use the conventional approach.

Recommendation 2.2: If you just want to access the outer scope’s content, use LET to distinguish `it` and `this` easier.

Taking the previous example of webviewSetting, assuming we need to access also the outer scope variable. If we use run, it will look like this

// ERROR
var javaScriptEnabled = false
var databaseEnabled = false

webviewSetting?.run {
javaScriptEnabled = javaScriptEnabled
databaseEnabled = databaseEnabled
}

This would confuse the compiler which is which. Of course you could rename it slightly and the compiler is smart enough to color code them as below, and it will compile. Nonetheless, it is still confusing for PR review etc.

So with this, it would be better to use below (assuming recommendation 2.1 above is met, where the variable is mutable)

// RECOMMENDED
var javaScriptEnabled = false
var databaseEnabled = false

webviewSetting?.let {
javaScriptEnabled = it.javaScriptEnabled
databaseEnabled = it.databaseEnabled
}

If using run, one could use this, but it is very confusing as shown below in differentiating which variable belongs to which scope.

// NOT RECOMMENDED
var javaScriptEnabled = false
var databaseEnabled = false

webviewSetting?.run {
javaScriptEnabled = this.javaScriptEnabled
databaseEnabled = this.databaseEnabled
}

Recommendation 2.3: When you have a long nullable chain with the front, use LET to eliminate the extra null `?` check.

Refer to the below function

// NOT RECOMMENDED
fun process(string: String?): List<Char>? {
return string?.asIterable()?.distinct()?.sorted()
}

Such a chain ? is not ideal, as it will introduce unnecessary null checking conditions that is not needed, because the first ? should have already eliminate the null result.

The decompile code looks as below

@Nullable
public final List process(@Nullable String string) {
List var2;
if (string != null) {
Iterable var10000 = StringsKt.asIterable(
(CharSequence)string);
if (var10000 != null) {
var2 = CollectionsKt.distinct(var10000);
if (var2 != null) {
var2 = CollectionsKt.sorted((Iterable)var2);
return var2;
}
}
}

var2 = null;
return var2;
}

Using a let would be as below, eliminating the ? and reduce the condition and complexity of the code (would be handy if you create unit test to score higher branch and complexity coverage)

Note: This is with the assumption you need to access the outer scope’s content as per point 2 above (note: I eliminate the outer scope’s content for brevity of explanation). Else use run would be better.
// RECOMMENDED
fun process(string: String?): List<Char>? {
return string?.let {
it.asIterable().distinct().sorted()
}
}

The decompile code looks as below

@Nullable
public final List process(@Nullable String string) {
List var10000;
if (string != null) {
int var4 = false;
var10000 = CollectionsKt.sorted(
(Iterable)CollectionsKt.distinct(
StringsKt.asIterable((CharSequence)string)));
} else {
var10000 = null;
}

return var10000;
}
Note: in some cases, the compiler optimized away the extra ?, e.g. when one just use filter, map etc. This case, no extra null check is performed, hence the decompiler code is the same (thanks to Lucio Maciel for pointing this out). Nonetheless, with let, it does reduces the number of ? needed in the code, hence still handy when we have a long nullable chain.

Recommendation 2.4: When you need to access the result, use LET to return the final result in the scope.

Refer to the below function

// NOT RECOMMENDED
fun process(stringList: List<String>?, removeString: String): Int? {
var count: Int? = null

if (stringList != null) {
count = stringList.filterNot{ it == removeString }
.map { it.length }
.sum()
}

return count
}

We need to get result of the chain i.e. the output of sum.

Despite the stringList is an immutable variable, which sounds like we should consider not using LET (Recommendation 1.1).

In this case, not using let will force us to create another variable for that purpose.

So , we could eliminate the additional variable code by using LET as below. Much concise and no extra variable needed.

// RECOMMENDED
fun process(stringList: List<String>?, removeString: String): Int? {
return stringList?.let {
it
.filterNot{ it == removeString }.map { it.length }.sum()
}
}

I hope this post is helpful to you. You could check out my other interesting topics here.

Follow me on medium, Twitter, Facebook or Reddit for little tips and learning on Android, Kotlin etc related topics. ~Elye~