Suppose we have a class declared as:
class Clock(var hours: Int, var minutes: Int)
you may define equality as:
override fun equals(other: Any?) = when (other) {
is Clock → other.hours == hours && other.minutes == minutes
else → false
}
where the first branch of when tests so-called structural equality (objects
with same content, same state).
Suppose now you want to optimize (of course it is not very useful here, just as
an example) and test first so-called referential equality, is it correct
to write:
override fun equals(other: Any?) = when (other) {
this → true
is Clock → other.hours == hours && other.minutes == minutes
else → false
}
1 Like
I believe that this would result in stack overflow since when uses the equals method to determine equality. Best is to use the definition generated by IntelliJ or even better to simply use a data class.
1 Like
Or if you really want to code it in more Kotlin style, the following should work:
when {
this === other -> true
this !is Clock -> false
this.hours != other.hours -> false
this.minutes != other.minutes -> false
else -> true
}
3 Likes
Yes, or perhaps simpler :
override fun equals(other: Any?) = when (other) {
this === other -> true
is Clock -> other.hours == hours && other.minutes == minutes
else -> false
}
The real kotlin style is to simply use data class.
2 Likes
No, this way is not correct. Here you compare other (of type Any?) with the result of this === other Boolean expression, which will most likely be false except when the other is boolean false. I believe this is not what you wanted.
Clearer to me now. Thanks a lot. The following seems ok then :
override fun equals(other: Any?) = when {
this === other → true
other is Clock → other.hours == hours && other.minutes == minutes
else → false
}
That still seems unnecessarily complex to me. I prefer to code equals operators like this:
override fun equals(other: Any?) =
this === other || (other is Clock && other.hours == hours && other.minutes == minutes)
Actually, I usually also omit the “this===other” clause, since it is only a speed optimization, and it is rare to compare an object to itself (in which case the clause saves time) and much, much more common to compare an object to another object (in which case the clause wastes time). This simplifies the code to:
override fun equals(other: Any?) =
other is Clock
&& other.hours == hours
&& other.minutes == minutes
which is very easy to read and maintain.
2 Likes
One other thing to keep in mind is that the situation changes if you have subclasses. For example:
open class Clock(
val hours: Int,
val minutes: Int
) {
override fun equals(other: Any?) =
(other is Clock && other.hours == hours && other.minutes == minutes)
}
class ClockWithSeconds(
hours: Int,
minutes: Int,
val seconds: Int
) : Clock(hours, minutes) {
override fun equals(other: Any?) =
(other is ClockWithSeconds && other.hours == hours
&& other.minutes == minutes && other.seconds == seconds)
}
val clock1 = Clock(5, 30)
val clock2 = ClockWithSeconds(5, 30, 11)
clock1 == clock2 // true
clock2 == clock1 // false
The pattern I like for this, if you don’t know what subclasses will exist, is to let the second object sanity-check that it can equal the first one, e.g.,
open class Clock(
val hours: Int,
val minutes: Int
) {
protected open fun canEqual(other: Any) =
javaClass.isAssignableFrom(other.javaClass)
override fun equals(other: Any?) =
(other is Clock && other.canEqual(this)
&& other.hours == hours && other.minutes == minutes)
}
class ClockWithSeconds(
hours: Int,
minutes: Int,
val seconds: Int
) : Clock(hours, minutes) {
override fun equals(other: Any?) =
(other is ClockWithSeconds && other.canEqual(this)
&& other.hours == hours && other.minutes == minutes
&& other.seconds == seconds)
}
val clock1 = Clock(5, 30)
val clock2 = ClockWithSeconds(5, 30, 11)
val clock3 = Clock(5, 30)
clock1 == clock2 // false
clock2 == clock1 // false
clock1 == clock3 // true
clock3 == clock1 // true
I like this better than checking for strict class equality because it’s more flexible: in some cases, you might want instances of subclasses to test equal to instances of superclasses, and you can adjust the canEqual logic appropriately.
After the big caveat that subclass equality is dangerous it can be done slightly more elegantly. Instead of having a second canEqual function you can do the following:
open class Clock {
/* something */
override fun equals(other: Any?):Boolean = when {
other===this -> true
other.class == Clock::class -> /* Regular clock equality check */
other is Clock -> other.equals(this)
/* This is always a child due to the previous check. It maintains symmetry by
* delegating to the most derived class. No extra methods. Child can choose to
* support equality or not.
*/
else -> false
}
class ClockWithSeconds(/*..*/):Clock(/*..*/) {
/* ... */
override fun equals(other: Any?):Boolean = when {
other===this -> true
other !is Clock -> false // to remove need for casts later
other is ClockWithSeconds -> /* Regular clock equality check */
other.class == Clock::class -> seconds == 0 && hours==other.hours && minutes == other.minutes
/* Could be the else condition if the class is final
*/
else -> false
}
2 Likes
Always delegating to the most derived class is a good insight. Nice.
If subclasses are involved equality is really tricky to get right. (It’s very easy to make subtle mistakes that won’t bite you immediately.)
I’d recommend reading this article by Martin Odersky et al, which is the best explanation and solution I’ve seen. (It’s adapted from their superb ‘Programming In Scala’ book, but in Java, and translates directly to Kotlin.)
However, if you don’t need to worry about subclasses, I find Kotlin lets me write it as a simple one-liner, e.g.:
override fun equals(other: Any?) = other === this || (other is ThisType && id == other.id)
(Obviously, replace ThisType, and add any other necessary fields. The other === this check is an optimisation you don’t strictly need, but it does no harm.)
By the way, I stumbled on an interesting article discussing some aspects of equals(), which I found much interesting : The infamous Any.equals
What do you think of its proposals ?
Nit-pick I believe you meant:
other !is Clock -> false