views:

103

answers:

4

I want to use a Dictionary(Of Key, Value), and the Key is not a base type, but a class like:

Public Class MyKey

    Sub New(ByVal packet As String, ByVal sent As Boolean)
        Me.packet = packet.ToUpper.Trim
        Me.sent = sent
    End Sub

    Private packet As String
    Private sent As Boolean
End Class

Now to have the Dictionary work and find the keys, I have to implement the System.IEquatable interface in the Key class (or use a different constructor, but that's another story):

Public Class MyKey
    Implements System.IEquatable(Of MyKey)


    Sub New(ByVal packet As String, ByVal sent As Boolean)
        Me.packet = packet.ToUpper.Trim
        Me.sent = sent
    End Sub

    Public Overloads Function Equals(ByVal other As MyKey) As Boolean Implements IEquatable(Of MyKey).Equals
        Return other.sent = Me.sent AndAlso other.packet = Me.packet
    End Function


    Private packet As String
    Private sent As Boolean
End Class

But to have consistent results I have also to implement Object.Equals and Object.GetHashCode:

Public Class MyKey
    Implements System.IEquatable(Of MyKey)


    Sub New(ByVal packet As String, ByVal sent As Boolean)
        Me.packet = packet.ToUpper.Trim
        Me.sent = sent
    End Sub

    Public Overloads Function Equals(ByVal other As ChiavePietanza) As Boolean Implements IEquatable(Of MyKey).Equals
        Return other.sent = Me.sent AndAlso other.packet = Me.packet
    End Function

    Overrides Function Equals(ByVal o As Object) As Boolean
        Dim cast As MyKey = DirectCast(o, MyKey)
        Return Equals(cast)
    End Function

    Public Overrides Function GetHashCode() As Integer
        Return packet.GetHashCode Or sent.GetHashCode
    End Function


    Private packet As String
    Private sent As Boolean
End Class

The question is: is that GetHashCode implementation correct? How should I implement it, to return a hashcode that kind of merges the string and the boolean hash codes?

+4  A: 

Your GetHashCode function is correct and conforms to the known rules of hash code implementations. In particular

  • It is the same for value for equivalent instances of MyKey
  • It does not change with changes to MyKey

The one way it could be made better though is to make both packet and sent be ReadOnly fields. Right now it is implied that they are ReadOnly because they are used in a GetHashCode function. However a future developer might overlook that, mutate the values and break the contract needed for GetHashCode. Having them explicitly ReadOnly prevents accidental breaks.

Another minor note. While it's a good idea to implement IEquatable(Of T) for Dictionary keys, it is not a requirement. All that is needed is to override the Equals and GetHashCode method in order to have a key properly function in a Dictionary.

JaredPar
do you think that XORing the two fields would be better?
vulkanino
@vulkanino, possibly and it is usually how GetHashCode is done. But it's impossible to say yes or no to that question with any certainty because it depends too much the real values of `packet` and `sent`. The best way to check is to run some tests with real data and seeing which method gives a better distribution of hash code values.
JaredPar
+2  A: 

Its okayish, but the Or operator doesn't generate well distributed hash values since it can only turn bits on, not off. Use the Xor operator instead.

Hans Passant
+1  A: 

This article is what I base all my GetHashCode overrides from:

SO - What is the best algorithm for an overridden System.Object.GetHashCode? So you could do something like this:

public override int GetHashCode() {
    int hash = 17;
    hash = (hash * 23) + ((packet != null) ? packet.GetHashCode() : 0);
    hash = (hash * 23) + ((sent != null) ? sent.GetHashCode() : 0);
    return hash;
}

It should be straight forward to port that to VB.

lumberjack4
I was in fact porting that class into VB, but can't set an "unchecked" block in VB :( and I get an OverflowException, ideas?
vulkanino
I think there is a flag under Advanced Compile Options that allows for unchecked arithmetic. Unfortunately I believe it is a project wide setting which may not be what you want.
lumberjack4
You could always wrap the code in a try-catch block, catch only the `OverflowException` and ignore it.
lumberjack4
@lumberjack4 but what the resulting integer will be??
vulkanino
Strike the above comment. Your best bet to get around this problem would be to either set the `Remove integer overflow checks` on the compiler settings or go with the `XOR` hash method.
lumberjack4
A: 

If you want something simple, and performance isn't critical, I believe that a dictionary of a structure type will regard two structures as equal if all their members are equal. Performance isn't very good, but the code is very simple.

supercat