so is there a consensus on: `if(x != null) y = x` ...
# announcements
c
so is there a consensus on:
if(x != null) y = x
vs
x?.let { y = x }
vs custom
ifNotNull(x) { y = it }
vs
y = x ?: y
? (I know, that the last one is not 100% equivalent to others as in case of property it invokes setter in any case)
1️⃣ 1
a
It depends on your programming style. In the particular case of assignment, the Elvis operator is always preferable. I think that I would otherwise use explicit if statements as null checks.
k
I'd day either of the first two is fine depending on style or the surrounding code, for example in a when statement I sometimes use the second to avoid having to add the
{}
to the case. I've never seen the last one, feel weird to me.
a
Check out kotlin style guidelines I believe there is an explicit mentioning that if (x != null) should be avoided https://kotlinlang.org/docs/reference/coding-conventions.html
🚫 1
g
There is no such mentioning there. They just say it's one of the many
.let
applications.
the downside of
let
is, it's easier to add
else
to the
if
version and it'll look more readable.
1
(WYSIWYG is piece of crap).
💯 1
a
Ok, I wasn't sure if they specifically mentioned use that instead of if checks. But the truth is without the else part just putting there if checks for null is taking from readability imo , basically code with no business logic behind it, if there is no else branch . If there is a need for else then you can't really avoid a null check. My style preference is to not invert the condition for the readability purposes meaning first put there if (x == null) //do something else do something else vs != null. Check this thread btw <https//discuss.kotlinlang.org/t/let vs if not null/3542|https//discuss.kotlinlang.org/t/let-vs-if-not-null/3542>
s
what’s so unreadable about a single null check? it’d be a different story if we were talking about a pyramid-of-doom sort of situation but if it only goes one layer deep,
if (x == null)
is far more readable and expressive than
x?.let { }
2
c
the use-case I'm reviewing is a simple entity update situation, where there's an entity
Copy code
class MyEntity {
  val id: Long? = null
  var prop1: String? = null
  var prop2: String? = null
// ...
  var propN: String? = null
}
and an update command:
Copy code
class UpdateEntityCommand {
  val myEntityId: Long
  var prop1: String? = null
  var prop2: String? = null
// ...
  var propN: String? = null
}
the handler should only change property of the entity if corresponding property of the command is not
null
:
Copy code
class MyHandler(private val repo: MyEntityRepository) {
  fun execute(command: UpdateEntityCommand) {
    with(repository.getOne(command.myEntityId)) {
      if (command.prop1 != null) prop1 = command.prop1 // (1)
      command.prop2?.let { prop2 = it }                // (2)
      ifNotNull(command.propN) { propN = it }          // (3)
    }
  }
}
By the way, detekt hates (2), and I have to
@Suppress("ComplexMethod")
a
@Shawn null checks (Java style ) in every method create noise imo feel with kotlin you can do better
s
let blocks that otherwise look identical to
if
statements “in every method” are just as noisy, if not noisier considering it’s less intuitive to read, and are potentially confusing if you’re doing anything side-effect-ey in the lambda. there are definitely times where let fits in well, e.g. chained monadic operations on a nullable, but this is not one of them. if you find yourself needing to something in a procedural way, I see no point in bending functional constructs to do so
k
@Czar Yeah I think that's one of the 2️⃣ cases.