override equals in Scala: no eq test ?
Francois
In Java, the idiomatic equals override looks like:
8<---------------------------------------------------------------
@Override public boolean equals(other:Object) {
if(other == this) return true;
if(null == other) return false;
if(this.getClass() != other.getClass()) return false;
final Test that = (Test) other;
//here, the actual logic
}
8<---------------------------------------------------------------
In Scala, it looks like[1]:
8<---------------------------------------------------------------
override def equals(other:Any) : Boolean = other match {
case that:Test => this.getClass == that.getClass &&
//actual logic
case _ => false
}
8<---------------------------------------------------------------
Is there a reason to not test for reference equality ? Something like:
8<---------------------------------------------------------------
override def equals(other:Any) = other match {
case x:AnyRef if x eq this => true
8<---------------------------------------------------------------
Perhaps the way pattern matching works make the (hypothetical)
performance gain disappear ?
The problem seems to be that we have an Any, and eq is only defined for
AnyRef, so we have to test for type at some point.
Or is there something like a special "case this =>...." (already
discussed in the past in the ml), for that very use case ?
Thanks,
[1] For example, in ProgInScala2, without canEquals
--
Francois ARMAND
http://fanf42.blogspot.com
http://www.normation.com
Paul Phillips
> In Scala, it looks like[1]:
> 8<---------------------------------------------------------------
> override def equals(other:Any) : Boolean = other match {
> case that:Test => this.getClass == that.getClass &&
> //actual logic
> case _ => false
> }
I hope not, or I missed quite the memo. Equality is performed that way
zero times in the compiler and standard library (and a good thing that
is) so if that's really in the book let's say it's a long and improbably
coherent printing error. Comparing class objects is not usually what
you want unless you like surprising the poor subclassers.
> override def equals(other:Any) = other match {
> case x:AnyRef if x eq this => true
If you want you can write this
other match { case _: this.type => true }
> Perhaps the way pattern matching works make the (hypothetical)
> performance gain disappear ?
It's unknown whether there would be a performance gain to
short-circuiting equals on reference equality, because it's a function
of how often it is successful compared to how long it takes to do the
check.
> The problem seems to be that we have an Any, and eq is only defined for
> AnyRef, so we have to test for type at some point.
Well, only because the implementation is mean. It's not like it doesn't
know Any erases to AnyRef and that everything is really an Object. I
think eq should be everywhere, or at least take an Any argument. You're
not actually doing a type "test" to get at eq because it will always
succeed, and test implies there is some uncertainty in the outcome.
You're really doing a "soothe the implementation" dance.
Bill Venners
On Sat, Mar 12, 2011 at 4:01 PM, Paul Phillips <pa...@improving.org> wrote:
> On 3/12/11 3:41 PM, Francois wrote:
>>
>> In Scala, it looks like[1]:
>> 8<---------------------------------------------------------------
>> override def equals(other:Any) : Boolean = other match {
>> case that:Test => this.getClass == that.getClass &&
>> //actual logic
>> case _ => false
>> }
>
> I hope not, or I missed quite the memo. Equality is performed that way zero
> times in the compiler and standard library (and a good thing that is) so if
> that's really in the book let's say it's a long and improbably coherent
> printing error. Comparing class objects is not usually what you want unless
> you like surprising the poor subclassers.
>
It isn't in PinS. I'm curious where you got your idea of an idiomatic
equals method Francois. The way it looks in PinS is:
class Rational(n: Int, d: Int) {
require(d != 0)
private val g = gcd(n.abs, d.abs)
val numer = (if (d < 0) -n else n) / g
val denom = d.abs / g
private def gcd(a: Int, b: Int): Int =
if (b == 0) a else gcd(b, a % b)
override def equals(other: Any): Boolean =
other match {
case that: Rational =>
(that canEqual this) &&
numer == that.numer &&
denom == that.denom
case _ => false
}
def canEqual(other: Any): Boolean =
other.isInstanceOf[Rational]
override def hashCode: Int =
41 * (
41 + numer
) + denom
override def toString =
if (denom == 1) numer.toString else numer +"/"+ denom
}
I believe in Effective Java Josh Bloch suggests a comparison with this
up front as a performance optimization. I think we didn't put such a
check into the book recommendation, because a) I didn't believe it
wasn't premature optimization and b) it would be a tad more hairy to
write in Scala because there's no eq method on Any.
Bill
>> override def equals(other:Any) = other match {
>> case x:AnyRef if x eq this => true
>
> If you want you can write this
>
> other match { case _: this.type => true }
>
>> Perhaps the way pattern matching works make the (hypothetical)
>> performance gain disappear ?
>
> It's unknown whether there would be a performance gain to short-circuiting
> equals on reference equality, because it's a function of how often it is
> successful compared to how long it takes to do the check.
>
>> The problem seems to be that we have an Any, and eq is only defined for
>> AnyRef, so we have to test for type at some point.
>
> Well, only because the implementation is mean. It's not like it doesn't
> know Any erases to AnyRef and that everything is really an Object. I think
> eq should be everywhere, or at least take an Any argument. You're not
> actually doing a type "test" to get at eq because it will always succeed,
> and test implies there is some uncertainty in the outcome. You're really
> doing a "soothe the implementation" dance.
>
--
Bill Venners
Artima, Inc.
http://www.artima.com
Francois
> I hope not, or I missed quite the memo. Equality is performed that way
> zero times in the compiler and standard library (and a good thing that
> is) so if that's really in the book let's say it's a long and improbably
> coherent printing error. Comparing class objects is not usually what you
> want unless you like surprising the poor subclassers.
Yes, and for the mailing list archive, the advised method to allow
subclassing is the use of a canEquals method:
8<---------------------------------------------------------------
override def equals(other: Any) = other match {
case that: Test => (that canEqual this) && ...
case _ => false
}
def canEqual(other: Any) =
other.isInstanceOf[Test] //override in subclass
8<---------------------------------------------------------------
>> override def equals(other:Any) = other match {
>> case x:AnyRef if x eq this => true
>
> If you want you can write this
>
> other match { case _: this.type => true }
What leads on my old 2.8.1 to "error: self type test in anonymous class
forbidden by implementation." But I note that I will be able to do that
in the future (and congrats to closing so hold bug as 266).
[...]
> Well, only because the implementation is mean. It's not like it doesn't
> know Any erases to AnyRef and that everything is really an Object. I
> think eq should be everywhere, or at least take an Any argument. You're
> not actually doing a type "test" to get at eq because it will always
> succeed, and test implies there is some uncertainty in the outcome.
> You're really doing a "soothe the implementation" dance.
Ok, thanks for that.
So for now, I will keep my simple test (without eq), I believe I can
live with a missed eq here and there for a shorter/cleaner code.
Thanks for the time and explanations,
Francois
> HI Francois& Paul,
[...]
> It isn't in PinS. I'm curious where you got your idea of an idiomatic
> equals method Francois.
OK, sorry to have brought that. The idiomatic, subclassing-proof equals
use the canEquals.
The one I pick is on p.694: "// A technically valid, but unsatisfying,
equals method"
class Point(val x: Int, val y: Int) {
override def hashCode = 41 * (41 + x) + y
override def equals(other: Any) = other match {
case that: Point =>
this.x == that.x && this.y == that.y &&
this.getClass == that.getClass
case _ => false
}
}
> I believe in Effective Java Josh Bloch suggests a comparison with this
> up front as a performance optimization. I think we didn't put such a
> check into the book recommendation, because a) I didn't believe it
> wasn't premature optimization and b) it would be a tad more hairy to
> write in Scala because there's no eq method on Any.
OK, that is a good answer, I will stick with that.
Thanks again,
