www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - How to use a class as key type in AAs?

reply Moritz Warning <moritzwarning web.de> writes:
I've read the docs and implemented size_t getHash, int opEquals(Object o) 
and int opCmp(Object o); but it still doesn't work.

The following code prints out: 2 2 1

Btw.: I tested (new Foo(1)) == (new Foo(1)) and it gives true,
(new Foo(1)) == (new Foo(2)) gives false.


void main()
{
	class Foo
	{
		uint i;
		this(uint i ) { this.i = i; }
 
		size_t getHash()
		{
			return this.i;
		}
 
		int opEquals(Object o)
		{
			auto t = cast(Foo) o;
			if(t is null) return false;
			return (this.i == t.i);
		}
 
		int opCmp(Object o)
		{
			return opEquals(o);
		}
	}
 
	uint[Foo] all;
 
	all[new Foo(1)] = 0;
	all[new Foo(2)] = 0;
	all[new Foo(2)] = 0;
 
	foreach(k, v; all)
	{
		Stdout(k.i).newline;
	}
}
Jul 06 2008
parent reply "Jarrett Billingsley" <kb3ctd2 yahoo.com> writes:
"Moritz Warning" <moritzwarning web.de> wrote in message 
news:g4r043$28f$1 digitalmars.com...

 I've read the docs and implemented size_t getHash, int opEquals(Object o)
 and int opCmp(Object o); but it still doesn't work.

 The following code prints out: 2 2 1

 Btw.: I tested (new Foo(1)) == (new Foo(1)) and it gives true,
 (new Foo(1)) == (new Foo(2)) gives false.


 void main()
 {
 class Foo
 {
 uint i;
 this(uint i ) { this.i = i; }

 size_t getHash()
 {
 return this.i;
 }

 int opEquals(Object o)
 {
 auto t = cast(Foo) o;
 if(t is null) return false;
 return (this.i == t.i);
 }

 int opCmp(Object o)
 {
 return opEquals(o);
 }
 }



You don't have opCmp implemented correctly.  opEquals returns true if they 
are equal, while opCmp returns 0.

It should be

int opCmp(Object o)
{
    auto t = cast(Foo)o;
    if(t is null)
        return 1;

    return i < t.i ? -1 : i > t.i ? 1 : 0;
}
Jul 06 2008
next sibling parent "Jarrett Billingsley" <kb3ctd2 yahoo.com> writes:
"Jarrett Billingsley" <kb3ctd2 yahoo.com> wrote in message 
news:g4r1c5$8b9$1 digitalmars.com...

 "Moritz Warning" <moritzwarning web.de> wrote in message 
 news:g4r043$28f$1 digitalmars.com...

 I've read the docs and implemented size_t getHash, int opEquals(Object o)
 and int opCmp(Object o); but it still doesn't work.

 The following code prints out: 2 2 1

 Btw.: I tested (new Foo(1)) == (new Foo(1)) and it gives true,
 (new Foo(1)) == (new Foo(2)) gives false.


 void main()
 {
 class Foo
 {
 uint i;
 this(uint i ) { this.i = i; }

 size_t getHash()
 {
 return this.i;
 }






And, it's supposed to be "toHash", not "getHash".  This is just where 
"override" comes in handy.
Jul 06 2008
prev sibling next sibling parent "Koroskin Denis" <2korden gmail.com> writes:
On Sun, 06 Jul 2008 22:02:44 +0400, Jarrett Billingsley  =

<kb3ctd2 yahoo.com> wrote:


 "Moritz Warning" <moritzwarning web.de> wrote in message
 news:g4r043$28f$1 digitalmars.com...

 I've read the docs and implemented size_t getHash, int opEquals(Objec=








 o)
 and int opCmp(Object o); but it still doesn't work.

 The following code prints out: 2 2 1

 Btw.: I tested (new Foo(1)) =3D=3D (new Foo(1)) and it gives true,
 (new Foo(1)) =3D=3D (new Foo(2)) gives false.


 void main()
 {
 class Foo
 {
 uint i;
 this(uint i ) { this.i =3D i; }

 size_t getHash()
 {
 return this.i;
 }

 int opEquals(Object o)
 {
 auto t =3D cast(Foo) o;
 if(t is null) return false;
 return (this.i =3D=3D t.i);
 }

 int opCmp(Object o)
 {
 return opEquals(o);
 }
 }



 You don't have opCmp implemented correctly.  opEquals returns true if =





 they
 are equal, while opCmp returns 0.

 It should be

 int opCmp(Object o)
 {
     auto t =3D cast(Foo)o;
     if(t is null)
         return 1;

     return i < t.i ? -1 : i > t.i ? 1 : 0;
 }



Slightly offtopic, but shouldn't we have some function (in std.intrinsic=
,  =

perhaps) to do just that:

int compare(int a, int b) {	// + overloads for byte, ubyte, short, ushor=
t,  =

uint, long, ulong
	return a < b ? -1 : a > b ? 1 : 0;
}
Jul 06 2008
prev sibling next sibling parent reply JAnderson <ask me.com> writes:
Jarrett Billingsley wrote:

 "Moritz Warning" <moritzwarning web.de> wrote in message 
 news:g4r043$28f$1 digitalmars.com...

 I've read the docs and implemented size_t getHash, int opEquals(Object o)
 and int opCmp(Object o); but it still doesn't work.

 The following code prints out: 2 2 1

 Btw.: I tested (new Foo(1)) == (new Foo(1)) and it gives true,
 (new Foo(1)) == (new Foo(2)) gives false.


 void main()
 {
 class Foo
 {
 uint i;
 this(uint i ) { this.i = i; }

 size_t getHash()
 {
 return this.i;
 }

 int opEquals(Object o)
 {
 auto t = cast(Foo) o;
 if(t is null) return false;
 return (this.i == t.i);
 }

 int opCmp(Object o)
 {
 return opEquals(o);
 }
 }



 You don't have opCmp implemented correctly.  opEquals returns true if they 
 are equal, while opCmp returns 0.
 
 It should be
 
 int opCmp(Object o)
 {
     auto t = cast(Foo)o;
     if(t is null)
         return 1;
 
     return i < t.i ? -1 : i > t.i ? 1 : 0;
 } 
 
 



Or just

int opCmp(Object o)
{
     auto t = cast(Foo)o;
     if(t is null)
         return 1;

     return i - t.i;
}

-Joel
Jul 06 2008
parent reply "Koroskin Denis" <2korden gmail.com> writes:
On Sun, 06 Jul 2008 22:26:46 +0400, JAnderson <ask me.com> wrote:


 Jarrett Billingsley wrote:

 "Moritz Warning" <moritzwarning web.de> wrote in message  =







 news:g4r043$28f$1 digitalmars.com...

 I've read the docs and implemented size_t getHash, int opEquals(Obje=











 o)
 and int opCmp(Object o); but it still doesn't work.

 The following code prints out: 2 2 1

 Btw.: I tested (new Foo(1)) =3D=3D (new Foo(1)) and it gives true,
 (new Foo(1)) =3D=3D (new Foo(2)) gives false.


 void main()
 {
 class Foo
 {
 uint i;
 this(uint i ) { this.i =3D i; }

 size_t getHash()
 {
 return this.i;
 }

 int opEquals(Object o)
 {
 auto t =3D cast(Foo) o;
 if(t is null) return false;
 return (this.i =3D=3D t.i);
 }

 int opCmp(Object o)
 {
 return opEquals(o);
 }
 }











 they are equal, while opCmp returns 0.
  It should be
  int opCmp(Object o)
 {
     auto t =3D cast(Foo)o;
     if(t is null)
         return 1;
      return i < t.i ? -1 : i > t.i ? 1 : 0;
 }



 Or just

 int opCmp(Object o)
 {
      auto t =3D cast(Foo)o;
      if(t is null)
          return 1;

      return i - t.i;
 }

 -Joel



Be careful, it is fast but gives wrong result when comparing int.min and=
  =

int.max!
I personally think that opCmp should return -1, 0 or 1 *only*. That's bo=
th  =

safer and allows more advanced tricks.
Jul 06 2008
parent reply JAnderson <ask me.com> writes:
Koroskin Denis wrote:

 On Sun, 06 Jul 2008 22:26:46 +0400, JAnderson <ask me.com> wrote:
 

 Jarrett Billingsley wrote:

 "Moritz Warning" <moritzwarning web.de> wrote in message 
 news:g4r043$28f$1 digitalmars.com...

 I've read the docs and implemented size_t getHash, int 
 opEquals(Object o)
 and int opCmp(Object o); but it still doesn't work.

 The following code prints out: 2 2 1

 Btw.: I tested (new Foo(1)) == (new Foo(1)) and it gives true,
 (new Foo(1)) == (new Foo(2)) gives false.


 void main()
 {
 class Foo
 {
 uint i;
 this(uint i ) { this.i = i; }

 size_t getHash()
 {
 return this.i;
 }

 int opEquals(Object o)
 {
 auto t = cast(Foo) o;
 if(t is null) return false;
 return (this.i == t.i);
 }

 int opCmp(Object o)
 {
 return opEquals(o);
 }
 }



 if they are equal, while opCmp returns 0.
  It should be
  int opCmp(Object o)
 {
     auto t = cast(Foo)o;
     if(t is null)
         return 1;
      return i < t.i ? -1 : i > t.i ? 1 : 0;
 }



 Or just

 int opCmp(Object o)
 {
      auto t = cast(Foo)o;
      if(t is null)
          return 1;

      return i - t.i;
 }

 -Joel



 Be careful, it is fast but gives wrong result when comparing int.min and 
 int.max!
 I personally think that opCmp should return -1, 0 or 1 *only*. That's 
 both safer and allows more advanced tricks.



True, but at least in my line of work using values in the min/max range 
is rare and you can always do something different if your using byte or 
unsigned.  Having to branch during this sort of operation can be very 
expensive since it can live in the innermost of loops.

-Joel
Jul 06 2008
parent reply Bill Baxter <dnewsgroup billbaxter.com> writes:
JAnderson wrote:

 Koroskin Denis wrote:

 On Sun, 06 Jul 2008 22:26:46 +0400, JAnderson <ask me.com> wrote:


 Jarrett Billingsley wrote:

 "Moritz Warning" <moritzwarning web.de> wrote in message 
 news:g4r043$28f$1 digitalmars.com...

 I've read the docs and implemented size_t getHash, int 
 opEquals(Object o)
 and int opCmp(Object o); but it still doesn't work.

 The following code prints out: 2 2 1

 Btw.: I tested (new Foo(1)) == (new Foo(1)) and it gives true,
 (new Foo(1)) == (new Foo(2)) gives false.


 void main()
 {
 class Foo
 {
 uint i;
 this(uint i ) { this.i = i; }

 size_t getHash()
 {
 return this.i;
 }

 int opEquals(Object o)
 {
 auto t = cast(Foo) o;
 if(t is null) return false;
 return (this.i == t.i);
 }

 int opCmp(Object o)
 {
 return opEquals(o);
 }
 }



 if they are equal, while opCmp returns 0.
  It should be
  int opCmp(Object o)
 {
     auto t = cast(Foo)o;
     if(t is null)
         return 1;
      return i < t.i ? -1 : i > t.i ? 1 : 0;
 }



 Or just

 int opCmp(Object o)
 {
      auto t = cast(Foo)o;
      if(t is null)
          return 1;

      return i - t.i;
 }

 -Joel



 Be careful, it is fast but gives wrong result when comparing int.min 
 and int.max!
 I personally think that opCmp should return -1, 0 or 1 *only*. That's 
 both safer and allows more advanced tricks.



 True, but at least in my line of work using values in the min/max range 
 is rare and you can always do something different if your using byte or 
 unsigned.  Having to branch during this sort of operation can be very 
 expensive since it can live in the innermost of loops.



Probably in most any line of work if you're getting within a factor of 
two of the max int then you are asking for trouble.

But I think the bigger issue is that precisely because you will never 
hit int.max or int.min, these values are often used as special indicator 
values.  Like to mean "not found" or "not initialized" or somesuch.

--bb
Jul 06 2008
parent JAnderson <ask me.com> writes:
Bill Baxter wrote:

 JAnderson wrote:

 Koroskin Denis wrote:

 On Sun, 06 Jul 2008 22:26:46 +0400, JAnderson <ask me.com> wrote:


 Jarrett Billingsley wrote:

 "Moritz Warning" <moritzwarning web.de> wrote in message 
 news:g4r043$28f$1 digitalmars.com...

 I've read the docs and implemented size_t getHash, int 
 opEquals(Object o)
 and int opCmp(Object o); but it still doesn't work.

 The following code prints out: 2 2 1

 Btw.: I tested (new Foo(1)) == (new Foo(1)) and it gives true,
 (new Foo(1)) == (new Foo(2)) gives false.


 void main()
 {
 class Foo
 {
 uint i;
 this(uint i ) { this.i = i; }

 size_t getHash()
 {
 return this.i;
 }

 int opEquals(Object o)
 {
 auto t = cast(Foo) o;
 if(t is null) return false;
 return (this.i == t.i);
 }

 int opCmp(Object o)
 {
 return opEquals(o);
 }
 }



 if they are equal, while opCmp returns 0.
  It should be
  int opCmp(Object o)
 {
     auto t = cast(Foo)o;
     if(t is null)
         return 1;
      return i < t.i ? -1 : i > t.i ? 1 : 0;
 }



 Or just

 int opCmp(Object o)
 {
      auto t = cast(Foo)o;
      if(t is null)
          return 1;

      return i - t.i;
 }

 -Joel



 Be careful, it is fast but gives wrong result when comparing int.min 
 and int.max!
 I personally think that opCmp should return -1, 0 or 1 *only*. That's 
 both safer and allows more advanced tricks.



 True, but at least in my line of work using values in the min/max 
 range is rare and you can always do something different if your using 
 byte or unsigned.  Having to branch during this sort of operation can 
 be very expensive since it can live in the innermost of loops.



 Probably in most any line of work if you're getting within a factor of 
 two of the max int then you are asking for trouble.
 
 But I think the bigger issue is that precisely because you will never 
 hit int.max or int.min, these values are often used as special indicator 
 values.  Like to mean "not found" or "not initialized" or somesuch.



This is true.  Its difficult to imagine many cases with int where people 
want to use the entire range of signed int for a container.  Maybe if 
they are storing a mask or something.  I guess a healthy dose of asserts 
would be handy.


 
 --bb
Jul 07 2008
prev sibling parent Moritz Warning <moritzwarning web.de> writes:
On Sun, 06 Jul 2008 14:02:44 -0400, Jarrett Billingsley wrote:


 "Moritz Warning" <moritzwarning web.de> wrote in message
 news:g4r043$28f$1 digitalmars.com...

 I've read the docs and implemented size_t getHash, int opEquals(Object
 o) and int opCmp(Object o); but it still doesn't work.

 The following code prints out: 2 2 1

 Btw.: I tested (new Foo(1)) == (new Foo(1)) and it gives true, (new
 Foo(1)) == (new Foo(2)) gives false.


 void main()
 {
 class Foo
 {
 uint i;
 this(uint i ) { this.i = i; }

 size_t getHash()
 {
 return this.i;
 }

 int opEquals(Object o)
 {
 auto t = cast(Foo) o;
 if(t is null) return false;
 return (this.i == t.i);
 }

 int opCmp(Object o)
 {
 return opEquals(o);
 }
 }



 You don't have opCmp implemented correctly.  opEquals returns true if
 they are equal, while opCmp returns 0.
 
 It should be
 
 int opCmp(Object o)
 {
     auto t = cast(Foo)o;
     if(t is null)
         return 1;
 
     return i < t.i ? -1 : i > t.i ? 1 : 0;
 }



Thanks for spotting the bug. :)
Jul 06 2008