Braces : block code and lambda confusion

Starting with this example:

class A {
	var prop: A? = null

	fun doX() {
		if(prop!=null) prop!!.print()
		else println("prop is null")
	}

	fun print(){
		println("...")
	}
}

We are trying to eliminate the use of !! (NPE hunt), so we rewrite doX():

	fun doX() {
		prop?.also { p ->
			p.print()
		} ?: println("prop is null")
	}

All is fine. Now, in the else case we need to add a second line.

With the if/else syntax we write:

	fun doXx() {
		if(prop!=null) prop!!.print()
		else {
			println("prop is null")
			println("second line")
		}
	}

Naturally, with the elvis we would write:

	fun doX() {
		prop?.also { p ->
			p.print()
		} ?: {
			println("prop is null")
			println("second line")
		}
	}

But this code is broken! The elvis right part is never called because braces means here a lambda and not a block code as with the “else” syntax.

Subtle bug, invisible and without any warning. Moreover this code has no sense since the two lines of codes in the lambda will never be called.

In this case, would it be possible to add a warning with an intention to rewrite it correctly ?

The right code is:

	fun doX() {
		prop?.also { p ->
			p.print()
		} ?: run {
			println("prop is null")
			println("second line")
		}
	}

This will prevent us from producing erroneous code that is not easy to detect and debug.

Thanks.

1 Like

I believe the return value checker will give you a warning on this

1 Like

As @kyay10 already answered the question, just as a pragmatic side-note: I found that in these situations when is usually better than if, especially as you can redefine the value as a val and therefore get rid of the nullability:

fun doX(): Unit =
    when(val p = prop) {
        null -> {
            println("null")
            println("second line")
        }
        else -> p.print()
    }

In my opinion, this is more readable than the Elvis version.

4 Likes

Unfortunately, no.

With -Xreturn-value-checker=check (or `=full`), a new warning is shown in formatGreeting() example, but no warning for the lambda never called.

1 Like

This code example should be indeed flagged with a warning.

Tracking issue: https://youtrack.jetbrains.com/issue/KT-77726

1 Like

I mean, the example is very contrived ; nothing in here seems natural. But IIUC, the issue is that ?: does not warn with value is unused ?

Maybe I’d showcase it with :

fun foo(ni : Int?, i : Int) {
  ni ?: i
}

This doesn’t warn, though certainly the value is unused, and it’s the same issue as the original post. This has nothing to do with lambdas right ?

I think this one doesn’t make sense, because the Elvis operator is irrelevant. If you remove it, you still have a statement that does nothing. It’s only relevant if the Elvis operator confuses the compiler and makes it not realise that the statement is unused.

fun foo(ni : Int?, i : Int) {
  ni // <--- does nothing, does compiler warn?
}
fun foo(ni : Int?, i : Int) {
  ni ?: i // <--- does nothing, does compiler warn?
}

Yes that was my point. The lone unused variable warns, but with the elvis it doesn’t. I think this is a much clearer version of the issue in the original post.

2 Likes

perhaps a stupid question, but why will the lambda never be called?
And what is the problem in formatGreeting() shown on the ‘+’ ?

The lambda will never be called, because it’s never called. :stuck_out_tongue:

The basic problem is that the Elvis operator provides an alternative value. Let’s look at a super basic example.

fun aOrB(a: Int?, b: Int): Int {
    return a ?: b
}

In this example, we have a function that takes two Integers, and returns an Integer. One of those Integers can be null. If a is not null, we return a. If a is null, we return b.

Basically, any expression (something that returns a value) can use an Elvis operator to say “if the first thing is null, then here, take this second thing”.

We could also write that example like so.

fun aOrB(a: Int?, b: Int): Int {
    if (a != null) {
        return a
    } else {
        return b
    }
}

The other thing to keep in mind is that an Elvis is like an if, in that whatever code is after the Elvis is ONLY run if the thing before the Elvis is null. Again, an example.

fun getValue(value: Int): Int {
    println("Returning value: $value")
    return value
}

fun aOrB(a: Int, b: Int): Int {
    if (getValue(a) > 0 || getValue(b) < 0) {
        return a
    } else {
        return b
    }
}
fun getValue(value: Int): Int {
    println("Returning value: $value")
    return value
}

fun aOrB(a: Int?, b: Int): Int {
    return getValue(a) ?: getValue(b)
}

In that first example, if a was 0, then it would print out “Returning value 0”, and “Returning value 5”. (Assuming b is 5) In the second example, if a is null, it would print out “Returning value null”, and “Returning value 5”. (Assuming b is 5) However, if in the first example, a is 1, it would print out “Returning value 1”, and nothing else. In the second example, if a is 1, it would print out “Returning value 1”, and nothing else.

Now, the final thing to know is that in the provided example, it’s using that behaviour of evaluating the code after the Elvis IF the code before the Elvis is null.

	fun doX() {
		prop?.also { p ->
			p.print()
		} ?: {
			println("prop is null")
			println("second line")
		}
	}

In this case, the Elvis expression is evaluated to return a value. Assuming prop is not null, the also function is called with a lambda. The also function runs, and then returns prop. If prop is null, then the code after the Elvis is run, which in this case, creates a lambda. That created lambda is then returned.

However, the returned value isn’t being used. It’s not being returned from the method, and it’s not being assigned to a variable. This is why this code works; if you were to try and assign it to a variable, the variable’s type would have to be Any, because the return could either be A, or () -> Unit.

It’s basically the same as if you were to write this code:

fun doX() {
    if ((prop != null && { prop!!.print(); true }.invoke()) || { println("prop is null"); println("second line) }) {
    }
}
1 Like