Re: Problem with equals with case class that extends Proxy
Nick D'Arcy
case class Test(a:String) extends Proxy {
def self = a
}
object Scratch {
def main(args:Array[String]) {
val label = new Test("bla")
println(label == label) // returns false!!!
println(label == "bla") // returns true
}
}
Hi all.
We've just moved our code base from 2.8.1 to 2.9.0-1 and this code doesn't do what you would expect:
import swing.Label
import scala.swing.Swing._
case class Test(a:String) extends Label
object Scratch {
def main(args:Array[String]) {
onEDT({
val label = new Test("bla")
println(label == label) // returns false!!!!
})
}
}
Is this a regression?
Thanks, Nick.
Thomas Sant'ana
> In fact, looking into this further, it seems to be a problem with case classes extending Proxy:
>
> case class Test(a:String) extends Proxy {
> def self = a
> }
>
> object Scratch {
> def main(args:Array[String]) {
> val label = new Test("bla")
> println(label == label) // returns false!!!
> println(label == "bla") // returns true
> }
>
Which begs the question, is this a regression?
I believe that for sanity, obj == obj must alway be true.
This appears causing alo kinds of issues with specs and mockito.
Thomas
Philippe Lhoste
> I believe that for sanity, obj == obj must alway be true.
Try "a".r == "a".r in the REPL... (= false for 2.9 at least).
--
Philippe Lhoste
-- (near) Paris -- France
-- http://Phi.Lho.free.fr
-- -- -- -- -- -- -- -- -- -- -- -- -- --
Thomas Sant'ana
On 13/07/2011 12:58, Thomas Sant'ana wrote:Try "a".r == "a".r in the REPL... (= false for 2.9 at least).
I believe that for sanity, obj == obj must alway be true.
Forgot to copy the list
Having a counter example doesn't make it necessarily true.
Effective Java second edition Item 8, suggests you respect the "equals" contract when you override it. This includes:
- Reflexive = x.equals(x) must be true
- Symmetric = x.equals(y) iff y.equals(x).
- Transitive
- Consistent
Default equals matches the object reference. Traditionally the override in scala respected this contract.
As of scala 2.9.0 classes that have proxy change the contract, and this does seem to cause changes of other libraries like Mockito, which rely on equals for several things. This may also cause issues with collections.
Example, of change:
scala> case class Test(x:String) extends Proxy { def self = x }
defined class Test
scala> val t = Test("a")
t: Test = a
scala> t == Test("a")
res0: Boolean = false
scala> val m = Map(t->1)
m: scala.collection.immutable.
scala> m.isDefinedAt(t)
res1: Boolean = true
scala> m.isDefinedAt(Test("a"))
res2: Boolean = false
On scala-2.8.0, this works:
scala> m.isDefinedAt(t)
res1: Boolean = true
scala> m.isDefinedAt(Test("a"))
res2: Boolean = true
This can break a lot of libraries.
Thomas
Daniel Sobral
> Em terça-feira, 12 de julho de 2011, Nick D'Arcy<nick...@gmail.com> escreveu:
>> In fact, looking into this further, it seems to be a problem with case classes extending Proxy:
>>
>> case class Test(a:String) extends Proxy {
>> def self = a
>> }
>>
>> object Scratch {
>> def main(args:Array[String]) {
>> val label = new Test("bla")
>> println(label == label) // returns false!!!
>> println(label == "bla") // returns true
>> }
>>
>
> Which begs the question, is this a regression?
>
> I believe that for sanity, obj == obj must alway be true.
Well, there's a big warning on Proxy that it may lead to asymmetric
equality, and that that's a bad thing.
What Proxy does is delegate comparison to some member. For instance:
class C(val x: Int) extends Proxy {
def self = x
}
This will cause new C(5) == 5 to be true. C(5) will also be equal to
C(5) here... the problem is not the use of Proxy alone.
The thing is that case classes also include Equals, which defines the
canEqual method. The purpose of that method is to stop subclasses from
being equal to superclasses, unless specifically told so. And, for
case classes, the answer is always "no".
So, if you are going to use Proxy to enable asymmetric equality, and
you are going to use anything that defines Equals, *you must override
canEqual*. The problem is not Proxy nor Equals -- case classes are not
meant to be subclassed, and if you do so, you have to deal with the
issues that arise from doing so.
--
Daniel C. Sobral
I travel to the future all the time.
Seth Tisue
>> I believe that for sanity, obj == obj must alway be true.
>
> Well, there's a big warning on Proxy that it may lead to asymmetric
> equality, and that that's a bad thing.
It warns you about asymmetry, but it doesn't warn you about a loss of
reflexivity!
I haven't thought it through, but might this be fixable by adding
canEquals to the list of methods Proxy proxies?
--
Seth Tisue | Northwestern University | http://tisue.net
lead developer, NetLogo: http://ccl.northwestern.edu/netlogo/
Daniel Sobral
> On Wed, Jul 13, 2011 at 11:21 AM, Daniel Sobral <dcso...@gmail.com> wrote:
>>> I believe that for sanity, obj == obj must alway be true.
>>
>> Well, there's a big warning on Proxy that it may lead to asymmetric
>> equality, and that that's a bad thing.
>
> It warns you about asymmetry, but it doesn't warn you about a loss of
> reflexivity!
>
> I haven't thought it through, but might this be fixable by adding
> canEquals to the list of methods Proxy proxies?
We should talk about it tomorrow. Note that the change that broke
things for people who have case classes extending equality was one of
the big equality fixes paulp did, to get stuff like 1 == BigDecimal(1)
working.
Nick D'Arcy
Any more news on this?
As far as I can tell there are only two options (please correct me if I'm wrong):
1) Say that case classes can't really extend the Proxy class and have the compiler output a warning much like it does at the moment when a case class extends another case class.
2) Update the Proxy class (and maybe other classes) so that case classes behave as expected when extending Proxy.
Thanks, Nick.
Daniel Sobral
> Hi guys.
>
> Any more news on this?
Yes. Everyone agree 2.9.0's behavior change was unintended and undesired.
> As far as I can tell there are only two options (please correct me if I'm
> wrong):
>
> 1) Say that case classes can't really extend the Proxy class and have the
> compiler output a warning much like it does at the moment when a case class
> extends another case class.
>
> 2) Update the Proxy class (and maybe other classes) so that case classes
> behave as expected when extending Proxy.
It is rather difficult to fix it, actually. Note, however, that the
classes that seemed to call for this change all actually use a
Proxy.Typed, which extends Proxy and is new as well. Last I heard, the
solution seemed to break the dependency from Typed to Proxy, making it
a brand new proxying class (possibly private, to avoid this kind of
problem), and reverting changes to Proxy.
I distinctly heard Seth Tissue committing himself to doing some of
this stuff. :-)