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

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.

"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

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

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;
}

"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

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

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.

"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.