for people who come from languages with pattern-ma...
# getting-started
y
for people who come from languages with pattern-matching conditional statements (for example,
if let
in Rust and Swift), what's your favorite way of expressing this in Kotlin? 1. with scope functions:
Copy code
parseData(data)?.also { /* ... */ }
2. with flow-sensitive typing:
Copy code
val parsed = parseData(data)
if (parsed != null) { /* ... */ }
3. some other way?
1
feels more idiomatic but not quite first-class.
2️⃣ 3
e
1
s
In my case, I mostly use
also
when performing additional actions, such as logging. So I prefer option 2.
1
y
@Seop yeah that's my sentiment as well.
also
semantically feels like "this is an additional side-effect that we can easily remove in the future if we don't want this" (so exactly like logging), not "this is the control flow in this code snippet".
👍 3
(worse imho is using
let
for side-effects, which I see often)
1
v
Use
apply
if you don't like
also
and
let
? :-D
Or
run
c
apply
is for "I want to apply additional configuration to this object", I wouldn't use it for flow-control
Personally I'd stick to an
if
here, or maybe
?:
depending on the situation.
1
y
(me being anal again) the multitude of different specific methods here, each for a different receiver, makes this "second-class feeling" stronger, even though Kotlin std docs dedicate an entire chapter for Scope Functions
but Kotlin seems to prefer allowing more expressiveness at the cost of redundancy (for example,
takeUnless
,
isNotEmpty
etc.) and I do appreciate that
c
For me: •
let
is for pure functions that convert data.
let
shouldn't be more than a single function call (
.let { it + 2 }
,
.let { it.toString() }
,
.let(::toDto)
,
.let(Instant::parse)
) •
?.let
should be avoided (otherwise people create long chains where it's impossible to know what can be null or not). •
also
is for side effects that are unrelated to the primary goal of the function. For example, logging, notifying an other system… All
also
calls in a function can be removed and the function should serve the same purpose. •
?.also
is ok. • There should never be an
also
chain. •
apply
is to apply additional configuration to an object you just created or got from elsewhere (
ArrayList<Int>().apply { add(5) }
, but there is
buildList
for that purpose now). •
run
without receiver is for declaring lexical scopes (to delete local variables before the end of a long function) or to have more complex logic in a field's initialization (
val a = run { … }
) •
run
with receiver I never really use •
takeIf
/`takeUnless` exist to ignore a subset of the possible values of a type before passing a value to another function.
"foo".indexOf('a').takeIf { it >= 0 }
because the
-1
case is illegal and we don't want it.
💯 3
3
y
just wanted to add that scope functions with receiver can be a footgun. for example, `.run`/`?.run`. if the closure compiles whether or not the receiver is null, it's somewhat easy to have the closure run (when you don't want it to) or not run (when you do) things like
val a = run { ... }
or
val b = foo()?.bar()?.baz() ?: run { ... }
are less error-prone
t
for me seeing things similar to
?: run { ... }
in a PR is almost always going to be a merge blocker. Using nullability chaining + scope functions can lead to some really hard to read and/or refactor code. Sure its less characters to type out, but what a foot gun it can grow into when your control flow doesn't get the explicit layout it probably should have gotten
c
I'm curious which situations you've seen with
?: run { … }
that cause bugs
Personnally I've seen a few times production bugs that were caused by
?.let {}?.let {}?.let {}
where a
null
happens in an unexpected place and the rest of the chain silently doesn't execute at all. Or a
?.let {}?.let {} ?: error("x not found")
where the error message is
x not found
even though the thing that was nullable was an entirely different thing
t
yeah thats exactly the type of issue that I've seen from these. whether intentional or not using the null chains you get into a habit of encoding arbitrary business concepts to be represented as null not giving yourself any characters to describe it to the next poor soul that has to read thru the code. So if any of
foo()?.bar()?.baz()
or the
let
blocks in your example change slightly its hard to spot that the meaning of a given null return may have changed and not mean what was originally represented by null.
1
feels great to write terse code that works. but you've got to include being able to read and understand it well in the future as part of the equation
y
but as mentioned above,
?: run { }
explicitly avoids null chaining, functioning as a clearly demarcated fallback case. I think it's fine as long as you don't chain more scope functions or elvis operators to this
run { }
. if you're okay with using
?:
at all then you trust the programmer to be able to understand if/when that closure is invoked. (and how else would you handle passing a block to the elvis operator instead of an expression, when Kotlin doesn't support block expressions?) I think the issue mentioned with null-chaining ("`foo()?.bar()?.baz()`"), is that adding or omitting a
?
can lead to unexpected short-circuiting. if not by you then by the next guy editing this code
t
yeah I have struggled to really come up with a clear explanation. I am totally fine with doing the simpler things like
?: error("bad thing happening")
or
someJsonData?.foo?.bar
to unwrap stuff to pass another function. most concise label i've been able to think of is calling it
nullability chain control flow
. really just a gut feeling measure of looking at the code and trying to determine if the nullability details are doing any heavy lifting. needing a scope block of code on an elvis operator is a dead giveaway that the null details are probably lifting more than they should be. very subjective. But tends to treat us better when you have 40+ people having to touch the same code over a few years.
👍 1
y
well, I must say I agree. complex control flow with scope functions just feels dirty. I try and avoid it and politely police coworkers to do the same (hence me opening this thread) when the syntax makes opening a new scope "heavier", people will naturally consider when it's actually appropriate to do so, instead of casually reaching for deep nesting or long call chains
t
the ones that have usually caused us trouble started out as small appropriate uses and grown over time for a quick bug fix here or there. For us at least its more of a cultural issue where people are less willing to restructure/refactor the code they are in when they think the fix could be small. gotten a lot better over the years for our teams, but i've had to be a broken record and constantly tell them roughly: "No code is holy, if it isn't good anymore just go ahead and change it"
y
I'm deeply familiar with that issue. I don't think I've ever managed to actually make a change here when it comes to other people.