Is the pattern `someNullableExpression?.let { thing -> thing.doStuff() } ?: run { otherwiseBehavi...

# announcements

[Travis Griggs]

Is the pattern someNullableExpression?.let { thing -> thing.doStuff() } ?: run { otherwiseBehavior() } a widely used pattern in Kotlin? I've been using it quite a bit when I port Swift code, because it's the closest (I thought) equivalent to Swift's if let thing = someNilableExpression { thing.doStuff } else { otherwiseBehavior(). But I've found edge cases where there are surprises, and so I've been rethinking whether I should be using it so much.

[Adam Powell]

Everyone who starts using kotlin goes nuts with this at about the 6-8 week mark and then regrets it a few months later. If you want an if/else, write an if/else.

[diesieben07]

Since Kotlin has smart-casting you can just use a normal if-else:

val thing = someNullableExpression()
if (thing != null) { thing.doStuff() } else { otherwiseBehavior() }

[Adam Powell]

right, but the reason to use the construct in the OP is to avoid thing being in the surrounding scope

[Adam Powell]

when (val foo = expression()) { is probably the closest analog in terms of that scoping

[diesieben07]

Well, one could argue that if that becomes an issue your functions are too large.
Alternatively you could also just do this:

someNullableExpression().let { thing ->
    if (thing != null) { thing.doStuff() } else { otherwiseBehavior() }
}

And then thing is contained within the let.

[Adam Powell]

yeah, the wrapper let is a good compromise. At that point the main tradeoff is the level of indentation 🙂

[Adam Powell]

and larger/smaller functions is always a question of colocating context. Small functions are great until you have to dig 6 degrees of separation away by ctrl-clicking in the IDE to find the code that actually does something, but once a function's entire body can't fit on a single screen without scrolling it gets unwieldy.

[Travis Griggs]

For me, the it's not so much polluting the context with a variable (though that could get to be an issue in big long methods which are discouraged anyway). It's that I want to evaluate and branch from one line, I think

[Travis Griggs]

IOW, the lure of the .let pattern over the if/else is that I could evaluate the expression and get a scoped variable bound in one line. whereas the if/else requires two lines (one for the evaluation) and the second for the null/branch test

[Travis Griggs]

But I am agreeing with you @Adam Powell. It seemed like such a good idea at first. A couple subtle bugs due to a trailing nested let tripping the outer run has definitely soured me on the pattern

[Nir]

note that if you want to leak scope, you can also use scope functions on the outside instead

[Nir]

e.g. you could do:

someNullableExpression.run {
    if (it == null) ...
    else ....
}

[Nir]

(* if you don't want to leak scope)

[Nir]

in this example you don't bind a variable called thing that is accessible throughout the function, and you clearly show exactly where the result of the expression is being used, without trying to stuff everything into one line

[Nir]

another advantage of this approach is that it guarantees smart casting to work, since you copied the variable into the argument of the scope function. If you nullable expression involves a var for example then this will sometimes cause smart casting to fail, since the variable could have been nulled in the meanwhile

[Travis Griggs]

As I type about this more and more and look at some code that just burned me. I don't actually mind the ?.let or ?.also. It's the trailing ?: run as a clever "else" that is the part that causes the problems

[Nir]

yeah, I mean usually the idea with ?: is more like using it as a default value

[Nir]

you could even chain a few ?: if you have short expressions and you simply want the first non-null one

[Nir]

but putting a longer block of code inside a ?: using run seems like not something you want to do really

[Travis Griggs]

So maybe the dilemma comes from @Adam Powell’s first statement, which I agree with a lot. I might posit the following to clarify it.

?let { } is a great one line replacement for when you want an if (no else) in the context of nullability testing. You get to eval, assign/bind, and conditionally evaluate, all in on terse expression.

But it doesn't scale to the case when you need an else, because trying to emulate a branch through chained messages will break down (because a chained send is not a branch). So when you want an if/else, use if/else

[Nir]

It's fine for "needing an else" in the sense of expressions, rather than long sequences of actions (which needed to be wrapped in run)

[Nir]

getSomething()?.let { transform(it) } ?: defaultValue is completely fine

[Travis Griggs]

right. but in the 68 cases I just found in our code where we've used this pattern, we always use it as a statement, never an expression. so then it gets tricky. because if your let block is just "do some work if this thing is nil, but you don't care about any transformative value, than you may accidentally return null (for example if your last line is another nested ?.let)

[Nir]

Yeah, I think using ?: as a statement is a bit suspect

[Nir]

probably adam's suggestion with creating a variable inside the when argument is really the best alternative here

[Nir]

you could also simply write your own letElse function I think:

someExpression?.let {
    ...
} letElse {
    ...
}

Or something like that. Perhaps write your own ifNull and ifNullElse. But obviously you'd question whether it was worth it to introduce this.

[Nir]

should be ifNotNull I suppose 🙂

[Adam Powell]

the problem stems from conflating null with the "take the else branch" directive

[Adam Powell]

you could write something like this using an inline class wrapping either the "true" block's return value or a bespoke "take the else branch" constant value used as a receiver by something like that letElse infix function

[Adam Powell]

but at some point you're just navel-gazing and making code that's hard for anyone else to read, like the people who used to define C macros like

#define BEGIN {
#define END }

[Nir]

whoa, let's take it down a notch. Nothing is as bad as doing that.

[Nir]

😉

[Nir]

i tend to agree though it's basically navel gazing, an extra level of indentation, or a variable slightly leaking in scope, neither of these are the end of the world in moderation

[Travis Griggs]

Thanks guys, this has been insightful and helpful. I like ?.let, but I'll be abandoning ?: run { } as my got to else solution. I just rewrote some code that did quite a bit of this, adopting a guard approach for the else/return stuff, and it actually made the code shorter