how can i make this conditional expression more elegantly with out `null == foo`？ ```if (null == foo...

# getting-started

[supra]

how can i make this conditional expression more elegantly with out `null == foo`？

if (null == foo || foo?.token == bar.token) { doSomeThing() }

[Chris Lee]

something like foo?.takeIf { it.token == bar.token }?.let { doSomething() }

[Joffrey]

Not sure that's better readability wise, especially since we apparently don't need foo. The initial code is actually very clear

[Chris Lee]

agree, the readability isn’t wonderful.

[Joffrey]

Also, it's actually not equivalent, doSomething should be run if foo is null

[supra]

I just think like @Chris Lee but it is not very well

[Chris Lee]

I tend to not use many if statements in my code, prefering when for clarity. Something like this reads better:

when {
  	foo == null -> doSomething()
  	foo.token == bar.token -> doSomething()
  }

[Chris Lee]

the op asked to remove foo == null which isn’t possible given the logic of that statement.

[Joffrey]

@supra I wouldn't usually go for .let if I'm making a side-effect like here. I find that if statements are better suited in this case.

[Michael de Kaste]

In the context, you're asking if
fieldA == null
or
fieldB == fieldC

the fact that fieldB is a subfield of a nonnullable fieldA doesn't matter for the fact that you are asking two seperate questions. I know that ?. and all implies a neat trick for nullable fields, but in this case it would be clearer to keep the two predicates as seperate entities

[Joffrey]

I agree with Michael. Anyway I think this discussion is not very objective and leans towards personal taste. I would simply write your initial expression like this (flipping the null check, and removing the safe call operator if possible):

if (foo == null || foo.token == bar.token) {
    doSomeThing()
}

[Joffrey]

It would be even better if you could make foo non-nullable, and rather express what null means for it in a more type-safe way, which would allow to clarify this condition

[supra]

I am agree with @Joffrey and @Michael de Kaste, but in fact, the null == foo is very likely to be accidentally deleted by colleagues.

[Joffrey]

Why so?

[supra]

@Joffrey cause by poor code review😂

[Joffrey]

I mean why would they want to delete this check?

[Michael de Kaste]

tests should cover this 😛

[Michael de Kaste]

deleting foo == null somewhere should fail a test somewhere

[Michael de Kaste]

we do have in our own project, the following extension functions:

inline fun <T> Boolean.ifTrue(block: () -> T): T? = if (this) block() else null
inline fun <T> Boolean.ifFalse(block: () -> T): T? = if (this) null else block()
inline fun <T> Boolean?.ifTrueOrNull(block: () -> T): T? = (this != false).ifTrue { block() }
inline fun <T> Boolean?.ifFalseOrNull(block: () -> T): T? = (this != true).ifTrue { block() }

so you can write something like:

foo?.token?.equals(bar.token).ifTrueOrNull{ doSomething() }

if that soothes your heart

[Michael de Kaste]

but then again you can do:

if(foo?.token?.equals(bar.token) ?: true){ doSomething() }

as well

[supra]

SO GREAT！thanks a lot with @Michael de Kaste，and also thanks @Joffrey @Chris Lee

[Joffrey]

I would really not recommend using elvis this way, it completely obfuscates the initial idea

[supra]

and i will also accept with null checking for simple and clean

[Joffrey]

If you have 2 distinct situations (e.g. NOT authenticated OR authenticated with the same token), the condition should express them both

[supra]

agree，the more elegantly solution i think is keep the null checking and cover this by unit testing

[Joffrey]

Also note that the safe call version above can mean something different if the token property of foo is itself nullable

[supra]

thanks @Joffrey i have already noticed this. the token is non null value

[Wout Werkman]

If you don't want your colleagues to remove the null check. You have even more reason to remove the unnecessary safe call. foo.token won't even compile if someone removes the foo == null check.
I think that the proposed if statement is already complete. If I were to think of a way to describe something like this in a utility method, I would make it something like:

if (foo.isNullOr { it.token == bar.token }) { doSomething() }

They both read the same: "if foo is null or it's token equals bar's token" vs your "if foo is null or *foo*'s token equals bar's token".
I do not say that this is objectively more readable at all. I just wanted to show what I personally use in case of chained calls such as application.service<UserRepository>().isNullOr { it.isInactive }. I think it's more explicit and it's easier to evaluate while debugging.

[Jacob]

if(foo?.token?.equals(bar.token) != false){ doSomething() }

seems elegant to me. It reads doSomething unless we have a clear discrepancy in the tokens between foo and bar

[Stephan Schröder]

btw. you no longer need the "?" in "foo?.token". The compiler will have smartcasted foo to be not-null at this point because it already cleared "null == foo", so

if (null == foo || foo.token == bar.token) { doSomeThing() }

will work.

[Joffrey]

Depends on what foo is, though. If it's a mutable property, that is not true

[Stephan Schröder]

@Joffrey correct, but in this case it'd be a good idea to make a local copy (or synchronized access) anyway.