am i using the dictinalry generic class correctly?

E

Elhanan

hi..

i wanted to build a Dictionary Classs that will load my own class
called letter,
i understood that i implement the IEquatable interface's equles method
that then the dictionary would use that inorder to compare the keys, so
i went ahead the implented it in the LETTER class so letter would
provice it's own means for comparison, but the dictionary ignored it

only when i created a new class, called it letterkey ,had it implement
EqualityComparer that will compare to classes , have it's GetHashCode
always return 0, only i managed to compare the two classes.

is that right? that's the only way i got it to work.

so now i have:

Dictionay<Letter,Letter> letters= new Dictionay<Letters,Letters>(new
LetterKey())
 
J

Jon Skeet [C# MVP]

Elhanan said:
hi..

i wanted to build a Dictionary Classs that will load my own class
called letter,
i understood that i implement the IEquatable interface's equles method
that then the dictionary would use that inorder to compare the keys, so
i went ahead the implented it in the LETTER class so letter would
provice it's own means for comparison, but the dictionary ignored it

Could you post a short but complete program which demonstrates the
problem?

See http://www.pobox.com/~skeet/csharp/complete.html for details of
what I mean by that.

Here's some example code which *does* work:

using System;
using System.Collections.Generic;

class Letter : IEquatable<Letter>
{
char value;

public Letter(char value)
{
this.value = value;
}

public bool Equals(Letter other)
{
Console.WriteLine ("Equals called");
return other.value==value;
}
}

class Test
{
static void Main()
{
Dictionary<Letter,Letter> map= new Dictionary<Letter,Letter>();

Letter c = new Letter('c');
map[c]=c;
Console.WriteLine (map[c]==c);
}
}
 
N

Nicholas Paldino [.NET/C# MVP]

Elhanan,

You need to have the IEquatable interface implementation return a hash
code. If you don't have it return a hash code for the instances passed in,
then you are pretty much killing the efficiency of the hash table. Since
all of the keys will return the same hashcode, only one bucket will be used,
and you will have to loop through all of the items to find the key.

Why not just return the hash value of the instances that are passed in?
 
E

Elhanan

IEquatable does not have a GetHashCode method, iEqualityComparaor does,
however when i simply returned the hash of the instance, i never got an
exception of a duplicatekey on the add method when i added to objects
which had the same key properties, which should make sense becoue each
object did have a different hashcode.
 
E

Elhanan

i didn't try to test the equality of the objects what is this:

Lettter l =new Letter (....)
Letter l2 =new (Letter(...) // same constructor datra

Dictionary<Letter,Letter> map= new Dictionary<Letter,Letter>();

map.add(l);
map.add(l2); // should a duplicae key exception here but didn''t.
 
N

Nick Hounsome

Elhanan said:
IEquatable does not have a GetHashCode method, iEqualityComparaor does,
however when i simply returned the hash of the instance, i never got an
exception of a duplicatekey on the add method when i added to objects
which had the same key properties, which should make sense becoue each
object did have a different hashcode.

I'm having trouble understanding you on this but note a few things:

1) If your class has a "standard" meaningful concept of equality then it
should
probably implement IEquatable and it should definitely override
Object.Equals and Object.GetHashCode such that if a.Equals(b) then
a.GetHashCode() == b.GetHashCode(). If 2 keys are "Equals()" but have
different hash codes then no hashtable will see them as being equal since
the comparison is essentially "a.GetHashCode() == b.GetHashCode() &&
a.Equals(b)" in that order.

2) If you do not give a good hashcode your dictionary/hashtable performance
will suck. In particular; If you return a constant value you will end up
doing linear searches of a linked list.

3) IEqualityComparer is should not be implemented by the target class itself
but by another class as it is intended for comparisons other than "normal".
For example Person.Equals would probably compare first and last names but
you might want a customer comparer that only compared surname. Sometime it
makes sense for a class to make a comparer available as a convenience using
a static method eg. Person.GetSurnameComparer()
 
E

Elhanan

ok as it stands my class does not implement IEquatable (actually i
think it does but it's not being used i think i'll drop it from what
you said).. it does not override GetHasCode or Object.Equels

in my case 2 letters are considered the same of certain group of their
properties are the same (but not all) for this i created a new class
callled letterkey that imlements iEqualityComparaor and overrode all
it's methodls, in it's compare method, i did the compare between the
classes and return true in case all of the properties i'm interested
are the same.
in it's GetHashCode i always return 0 becouse otherwise the Equels
method would never get called, so i have no choice.
 
J

Jon Skeet [C# MVP]

Elhanan said:
ok as it stands my class does not implement IEquatable (actually i
think it does but it's not being used i think i'll drop it from what
you said).. it does not override GetHasCode or Object.Equels

in my case 2 letters are considered the same of certain group of their
properties are the same (but not all) for this i created a new class
callled letterkey that imlements iEqualityComparaor and overrode all
it's methodls, in it's compare method, i did the compare between the
classes and return true in case all of the properties i'm interested
are the same.
in it's GetHashCode i always return 0 becouse otherwise the Equels
method would never get called, so i have no choice.

You do have a choice - return a value which will always be the same for
any two objects which will compare positively for equality. There are
plenty of ways of doing that, usually involving combining the hashcodes
of the properties you compare in the equality comparison.

If you're just returning 0 for all the hash codes, you should just use
a list instead...

Jon
 
E

Elhanan

ok i looked at the help some more and this what i did:

i overode my GetHashCode method in my function and implemented so it
would return all the properties with xor operator between them (most of
my properties are int anway, and one is date and the other is decima,
so for them i simply used their getHasCode method), it then implemented
IEquatable interface in my letter like so:
public bool Equals(Letter source)
{
return this.GetHashCode() == source.GetHashCode();
}

so now i don't think i need another class, i still get an actuall has
function right?
 
J

Jon Skeet [C# MVP]

Elhanan said:
ok i looked at the help some more and this what i did:

i overode my GetHashCode method in my function and implemented so it
would return all the properties with xor operator between them (most of
my properties are int anway, and one is date and the other is decima,
so for them i simply used their getHasCode method), it then implemented
IEquatable interface in my letter like so:
public bool Equals(Letter source)
{
return this.GetHashCode() == source.GetHashCode();
}

so now i don't think i need another class, i still get an actuall has
function right?

Well, firstly I wouldn't implement your hash code like that. A quick
and reasonable hash code is obtained using something like:

int ret = 17;
ret = 37*ret + FirstProperty.GetHashCode();
ret = 37*ret + SecondProperty.GetHashCode();
// etc
return ret;

The advantage of that over using XOR is this: Suppose you have two ints
in your object. If they have the same value, when they are XORed
together that result is 0 *whatever the value is*. In other words,
although they are different, they get the same hash code. Now, this
isn't fatal, but it doesn't make for a great hash code.


Secondly, your Equals method is broken - it will return true for
comparisons where the hash codes are the same but the objects are
different. There's nothing to say a hash code has to be unique, and
indeed very few are. You should compare your properties, just as you
did before.
 
E

Elhanan

do you mean to say that even though some of my properties that compose
the objects' unique properties will be diffrenet' their hashcode will
not be? how can that be i compose the hash from values?


btw how about the next idea for has code? if append all the properties
as string return the concatnated string's hashcode will that be ok?

i didnt' understand 17 and 37 numbers...
 
J

Jon Skeet [C# MVP]

Elhanan said:
do you mean to say that even though some of my properties that compose
the objects' unique properties will be diffrenet' their hashcode will
not be? how can that be i compose the hash from values?

Think about it - if you've got more than one int, then you've got more
values than can be expressed in a single int (which is what a hashcode
is). If you've got two ints (and nothing else) then you've got 2^64
possible combinations, but there are only 2^32 possible hashcodes.

A hashcode doesn't need to be unique - it's just meant to be a quick
way of narrowing down choices to a small set of possible matches.
btw how about the next idea for has code? if append all the properties
as string return the concatnated string's hashcode will that be ok?

I wouldn't do that - there seems little reason to. Using each hashcode
is fine (and more efficient than using string concatenation) - it's
just that using XOR isn't a great idea.
i didnt' understand 17 and 37 numbers...

They just happen to be prime numbers which work reasonably well to
produce hashcodes.
 
E

Elhanan

ok, i corrected the code of the equels, and getHashCode, seems to be
working.


while we are on the subject i was wondering can i bind the dictionary
class to a datagridview?

what i have now is class box which wraps around the Letters propety,
expsoes it's values collection as a property and has 3 other addtional
properties on (BoxNo, etc..) each time a new Letter Class is added, the
class first adds it to the collection and calls it's DataAccess Class
which opens up a connection, execute's an insert and closes the
connection. (same with delete, no Updateing properties becouse of the
dictionary key values off course).

i'm adding rows directly to the grid, but rather from an outside field
(it's a barcode string), one i leave the field, i parse the string turn
it into a letter class and adds it to the collection, the box number
throws out a DuplicateKey Exception if i allready added it .
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top