In a function like `equals`, when should you use `if(condition) { return true } else {}` and when sh...

# codingconventions

[Sam Stone]

In a function like equals, when should you use if(condition) { return true } else {} and when should you leave out the else, and use implicit flow control (it is by definition else because it didn’t return). Is there a performance impact in including else?

[Chris Lee]

personally I avoid else to the extent possible. in say equals() if would be if(condition) return false with objects being equal only if they make it all the way through the conditions. No performance change for implicit flow control (absence of else).

[ephemient]

else is fine (and necessary) if you lift the return out, e.g.

return if (condition) { true} else { ... }

and if you can get it all the way to the top level, you can define the function as = expression instead of { block }

[ephemient]

but otherwise, I generally wouldn't use else when it introduces unnecessary nesting, unless it is specifically clearer in some circumstance

[ephemient]

generally equals can be defined by a simple expression though, e.g.

class Foo(val foo: String) {
    override fun equals(other: Any?): Boolean =
        other is Foo && this.foo == other.foo
    override fun hashCode(): Int = /* must override hashCode when overriding equals */

[Chris Lee]

generally a complete and correct equals can’t be represented in a single if expression:

override fun equals(other: Any?): Boolean {
    if (this === other) return true
    if (javaClass != other?.javaClass) return false

    other as OtherType

    if (url != other.url) return false
    if (value != other.value) return false

    return true
}

However, it can be collapsed into a when statement:

override fun equals(other: Any?): Boolean {
    return when {
        this === other -> true
        javaClass != other?.javaClass -> false
        other !is OtherType -> false
        url != other.url -> false
        else -> true
    }
}

[ephemient]

when the class is final (as the default in Kotlin is) that extra check is not required

[elizarov]

I consider it a bad style to have an empty else.

[Sam Stone]

@elizarov I didn't mean it should be empty, I meant that the rest of the function should be in the else.

[elizarov]

I consider this as a bad style, too.