www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - opCmp, opEquals

reply bearophile <bearophileHUGS lycos.com> writes:
I have a small question regarding OOP design. The signature of opCmp, opEquals
when defined inside classes require a Object argument.
So you have to define what to do when the given object can't be cast to the
this class. Here you can see two alternative designs:

http://codepad.org/tOHnTObl

I think the C1 design is a bad design and the C2 is the right one, but I'd like
to know your opinion.

I think Python 2.6+ uses the C1 design, Python3+ uses the C2 design.

Someone told me that Java uses a mixed approach: the opEquals returns false
when the cast is not possible, while the cmp throws an exception.

Bye,
bearophile
Oct 23 2008
next sibling parent reply "Jarrett Billingsley" <jarrett.billingsley gmail.com> writes:
On Thu, Oct 23, 2008 at 11:04 AM, bearophile <bearophileHUGS lycos.com> wrote:

 I have a small question regarding OOP design. The signature of opCmp, opEquals
when defined inside classes require a Object argument.
 So you have to define what to do when the given object can't be cast to the
this class. Here you can see two alternative designs:

 http://codepad.org/tOHnTObl

 I think the C1 design is a bad design and the C2 is the right one, but I'd
like to know your opinion.

 I think Python 2.6+ uses the C1 design, Python3+ uses the C2 design.

 Someone told me that Java uses a mixed approach: the opEquals returns false
when the cast is not possible, while the cmp throws an exception.

 Bye,
 bearophile


I think C2 is the right one as well.
Oct 23 2008
parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
"Jarrett Billingsley" wrote

 On Thu, Oct 23, 2008 at 11:04 AM, bearophile <bearophileHUGS lycos.com> 
 wrote:

 I have a small question regarding OOP design. The signature of opCmp, 
 opEquals when defined inside classes require a Object argument.
 So you have to define what to do when the given object can't be cast to 
 the this class. Here you can see two alternative designs:

 http://codepad.org/tOHnTObl

 I think the C1 design is a bad design and the C2 is the right one, but 
 I'd like to know your opinion.

 I think Python 2.6+ uses the C1 design, Python3+ uses the C2 design.

 Someone told me that Java uses a mixed approach: the opEquals returns 
 false when the cast is not possible, while the cmp throws an exception.

 Bye,
 bearophile


 I think C2 is the right one as well.


Given that the default implementation just compares references, I think C1 
is correct.

Otherwise you have weird shit like this:

class C3
{
  // no opEquals defined, very common
}

auto c3 = new C3;
auto c2 = new C2(0);

if(c3 == c2) // fails, but does not throw
{
    foo();
}

if (c2 == c3) // throws exception
{
   foo();
}

-Steve
Oct 23 2008
parent reply bearophile <bearophileHUGS lycos.com> writes:
Steven Schveighoffer:

 Given that the default implementation just compares references, I think C1 
 is correct.
 Otherwise you have weird shit like this:
 [...something bad]


In that C1 class I have left out the implementation of opCmp, let's say I use
the following one (C4a and C4b have the same code, but they are distinct
classes):

class UncomparableException: Exception {
    this() {
        super(this.classinfo.name);
    }
    this(string msg) {
        super(this.classinfo.name ~ ": " ~ msg);
    }
}


class C4a { // Like Java?
    int x;
    this(int x) { this.x = x; }

    int opEquals(Object o) {
        if (this is o)
            return 1;
        auto oc = cast(typeof(this))o;
        if (oc is null)
            return 0;
        else
            return this.x == oc.x;
    }

    int opCmp(Object o) {
        if (this is o)
            return 0;
        auto oc = cast(typeof(this))o;
        if (oc is null)
            throw new UncomparableException();
        else
            return this.x - oc.x;
    }
}


class C4b { // Like Java?
    int x;
    this(int x) { this.x = x; }

    int opEquals(Object o) {
        if (this is o)
            return 1;
        auto oc = cast(typeof(this))o;
        if (oc is null)
            return 0;
        else
            return this.x == oc.x;
    }

    int opCmp(Object o) {
        if (this is o)
            return 0;
        auto oc = cast(typeof(this))o;
        if (oc is null)
            throw new UncomparableException();
        else
            return this.x - oc.x;
    }
}

void main() {
    auto x = new C4a(1);
    auto y = new C4b(2);

    printf(x == y ? "x == y\n" : "x != y\n"); // else
    printf(x.opCmp(y) == 0 ? "x ==2 y\n" : "x !=2 y\n"); // throws
}

I think that's more bad behavior.
But then what can I do? remove the UncomparableException from opCmp too, and
make it return randomly -1 or +1 when the classes can't be cast, like this?


class C5 { // ?
    int x;
    this(int x) { this.x = x; }

    int opEquals(Object o) {
        if (this is o)
            return 1;
        auto oc = cast(typeof(this))o;
        if (oc is null)
            return 0;
        else
            return this.x == oc.x;
    }

    int opCmp(Object o) {
        if (this is o)
            return 0;
        auto oc = cast(typeof(this))o;
        if (oc is null)
            return 1; // random value != 0
        else
            return this.x - oc.x;
    }
}

Bye,
bearophile
Oct 23 2008
next sibling parent reply Ary Borenszweig <ary esperanto.org.ar> writes:
bearophile wrote:

 Steven Schveighoffer:

 Given that the default implementation just compares references, I think C1 
 is correct.
 Otherwise you have weird shit like this:
 [...something bad]


 
 In that C1 class I have left out the implementation of opCmp, let's say I use
the following one (C4a and C4b have the same code, but they are distinct
classes):
 
 class UncomparableException: Exception {
     this() {
         super(this.classinfo.name);
     }
     this(string msg) {
         super(this.classinfo.name ~ ": " ~ msg);
     }
 }
 
 
 class C4a { // Like Java?
     int x;
     this(int x) { this.x = x; }
 
     int opEquals(Object o) {
         if (this is o)
             return 1;
         auto oc = cast(typeof(this))o;
         if (oc is null)
             return 0;
         else
             return this.x == oc.x;
     }
 
     int opCmp(Object o) {
         if (this is o)
             return 0;
         auto oc = cast(typeof(this))o;
         if (oc is null)
             throw new UncomparableException();
         else
             return this.x - oc.x;
     }
 }
 
 
 class C4b { // Like Java?
     int x;
     this(int x) { this.x = x; }
 
     int opEquals(Object o) {
         if (this is o)
             return 1;
         auto oc = cast(typeof(this))o;
         if (oc is null)
             return 0;
         else
             return this.x == oc.x;
     }
 
     int opCmp(Object o) {
         if (this is o)
             return 0;
         auto oc = cast(typeof(this))o;
         if (oc is null)
             throw new UncomparableException();
         else
             return this.x - oc.x;
     }
 }
 
 void main() {
     auto x = new C4a(1);
     auto y = new C4b(2);
 
     printf(x == y ? "x == y\n" : "x != y\n"); // else
     printf(x.opCmp(y) == 0 ? "x ==2 y\n" : "x !=2 y\n"); // throws
 }
 
 I think that's more bad behavior.
 But then what can I do? remove the UncomparableException from opCmp too, and
make it return randomly -1 or +1 when the classes can't be cast, like this?


If you are comparing something to something else that you shouldn't be 
comparing, an exception is the best solution. It will fail fast, and 
show you that there is an error in your application code.

Another altenative, more in the D style, it to place an assert:

int opCmp(Object o) {
      if (this is o)
          return 0;

      assert(typeof(o) is typeof(this));

      auto oc = cast(typeof(this))o;
      return this.x - oc.x;
}
Oct 23 2008
parent Carl Clark <carlclark lavabit.com> writes:
On 2008-10-23 11:42, Ary Borenszweig wrote:

 bearophile wrote:

 Steven Schveighoffer:

 Given that the default implementation just compares references, I 
 think C1 is correct.
 Otherwise you have weird shit like this:
 [...something bad]


 In that C1 class I have left out the implementation of opCmp, let's 
 say I use the following one (C4a and C4b have the same code, but they 
 are distinct classes):

 class UncomparableException: Exception {
     this() {
         super(this.classinfo.name);
     }
     this(string msg) {
         super(this.classinfo.name ~ ": " ~ msg);
     }
 }


 class C4a { // Like Java?
     int x;
     this(int x) { this.x = x; }

     int opEquals(Object o) {
         if (this is o)
             return 1;
         auto oc = cast(typeof(this))o;
         if (oc is null)
             return 0;
         else
             return this.x == oc.x;
     }

     int opCmp(Object o) {
         if (this is o)
             return 0;
         auto oc = cast(typeof(this))o;
         if (oc is null)
             throw new UncomparableException();
         else
             return this.x - oc.x;
     }
 }


 class C4b { // Like Java?
     int x;
     this(int x) { this.x = x; }

     int opEquals(Object o) {
         if (this is o)
             return 1;
         auto oc = cast(typeof(this))o;
         if (oc is null)
             return 0;
         else
             return this.x == oc.x;
     }

     int opCmp(Object o) {
         if (this is o)
             return 0;
         auto oc = cast(typeof(this))o;
         if (oc is null)
             throw new UncomparableException();
         else
             return this.x - oc.x;
     }
 }

 void main() {
     auto x = new C4a(1);
     auto y = new C4b(2);

     printf(x == y ? "x == y\n" : "x != y\n"); // else
     printf(x.opCmp(y) == 0 ? "x ==2 y\n" : "x !=2 y\n"); // throws
 }

 I think that's more bad behavior.
 But then what can I do? remove the UncomparableException from opCmp 
 too, and make it return randomly -1 or +1 when the classes can't be 
 cast, like this?


 
 If you are comparing something to something else that you shouldn't be 
 comparing, an exception is the best solution. It will fail fast, and 
 show you that there is an error in your application code.
 
 Another altenative, more in the D style, it to place an assert:
 
 int opCmp(Object o) {
      if (this is o)
          return 0;
 
      assert(typeof(o) is typeof(this));
 
      auto oc = cast(typeof(this))o;
      return this.x - oc.x;
 }


I don't think assert is quite what you'd want here; it seems like an 
exception is really better, since asserts are more for invariant 
conditions -- the function doesn't know that no one will ever call it 
with bad arguments, so it shouldn't be an assert.
Oct 23 2008
prev sibling parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
"bearophile" wrote

 Steven Schveighoffer:

 Given that the default implementation just compares references, I think 
 C1
 is correct.
 Otherwise you have weird shit like this:
 [...something bad]


 In that C1 class I have left out the implementation of opCmp, let's say I 
 use the following one (C4a and C4b have the same code, but they are 
 distinct classes):
 [snip]



 void main() {
    auto x = new C4a(1);
    auto y = new C4b(2);

    printf(x == y ? "x == y\n" : "x != y\n"); // else
    printf(x.opCmp(y) == 0 ? "x ==2 y\n" : "x !=2 y\n"); // throws
 }

 I think that's more bad behavior.


Yes, but consistently bad ;)

that is, x.opCmp(y) results in the same thing (throwing an exception) as 
someObj.opCmp(y), where someObj's opCmp is the default implementation (note 
that this is currently broken in Tango, but not in druntime).

The real solution here is to have opCmp not be in object, but in an 
interface.  The fact that the default opCmp throws an exception is a huge 
clue to this.

But I doubt that's changing anytime soon.


 But then what can I do? remove the UncomparableException from opCmp too, 
 and make it return randomly -1 or +1 when the classes can't be cast, like 
 this?


The exception is the default behavior, so it should be what you use for the 
default case.

-Steve
Oct 23 2008
parent bearophile <bearophileHUGS lycos.com> writes:
Steven Schveighoffer:


that is, x.opCmp(y) results in the same thing (throwing an exception) as
someObj.opCmp(y), where someObj's opCmp is the default implementation [...]<
The exception is the default behavior, so it should be what you use for the
default case.<


Okay. D works like Java here, it seems. So I'll do the same then, thank you.



The real solution here is to have opCmp not be in object, but in an interface. 
The fact that the default opCmp throws an exception is a huge clue to this.<


I see, thank you for the idea.
If you think this is the best solution, then I think that now with the
refinment of druntime is the best moment to change things. Later this detail of
D2 will be too much frozen to change. So Sean may be asked to express an
opinion about this.

---------------------

Ary Borenszweig:


 int opCmp(Object o) {
       if (this is o)
           return 0;
       assert(typeof(o) is typeof(this));
       auto oc = cast(typeof(this))o;
       return this.x - oc.x;
 }


I like something like this better:

int opCmp(Object o) {
      if (this is o)
          return 0;
      auto oc = cast(typeof(this))o;
      assert(oc !is null);
      return this.x - oc.x;
}

But in the end I'll keep the runtime exception in my opCmp methods.

Thank to everyone that has given me a clue,
bye,
bearophile
Oct 23 2008
prev sibling parent "Stewart Gordon" <smjg_1998 yahoo.com> writes:
"bearophile" <bearophileHUGS lycos.com> wrote in message 
news:gdq3pq$126t$1 digitalmars.com...

I have a small question regarding OOP design. The signature of opCmp, 
opEquals when defined inside classes require a Object argument.
 So you have to define what to do when the given object can't be cast to 
 the this class. Here you can see two alternative designs:


<snip>

 Someone told me that Java uses a mixed approach: the opEquals returns 
 false when the cast is not possible, while the cmp throws an exception.


This seems sensible to me.  If two objects are of completely different 
classes, obviously they aren't equal to each other.  But there's no means by 
which one can be picked out as being 'less than' the other.

Stewart.

-- 
My e-mail address is valid but not my primary mailbox.  Please keep replies 
on the 'group where everybody may benefit.
Oct 28 2008