How can I improve this 31-bit SuperFastHash implementation in VBA?

Question

I'm implementing a variation of the SuperFastHash in VBA for use in Excel (32-bit version, so no LongLong available) to hash strings.

To get around the limitations of signed 32-bit Long values, I'm doing the addition and bit-shifting using Double types, and then converting from Double to Long in a way that truncates it at 31 bits (the maximum positive value -- don't want to deal with two's complement and signs).

I'm getting answers and avoiding overflows so far, but I have a suspicion I'm making some mistakes in translation, since most implementations use all 32 bits of a uint and also deal with individual bytes from an array rather than 16-bit values coming from AscW().

Any suggestions of improvement, especially of the string-handling, bit-shifting, or the final avalanche?

Public Function shr(ByVal Value As Long, ByVal Shift As Byte) As Long
    shr = Value
    If Shift > 0 Then shr = shr \ (2 ^ Shift)
End Function

Public Function shl(ByVal Value As Long, ByVal Shift As Byte) As Long
    If Shift > 0 Then
        shl = LimitDouble(CDbl(Value) * (2& ^ Shift))
    Else
        shl = Value
    End If
End Function

Public Function LimitDouble(ByVal d As Double) As Long
    '' Prevent overflow by lopping off anything beyond 31 bits
    Const MaxNumber As Double = 2 ^ 31
    LimitDouble = CLng(d - (Fix(d / MaxNumber) * MaxNumber))
End Function

Public Function SuperFastHash(ByVal dataToHash As String) As Long
    Dim dataLength As Long
    dataLength = Len(dataToHash)
    If (dataLength = 0) Then
        SuperFastHash = 0
        Exit Function
    End If
    Dim hash As Long
    hash = dataLength
    Dim remainingBytes As Integer
    remainingBytes = dataLength Mod 2
    Dim numberOfLoops As Integer
    numberOfLoops = dataLength \ 2
    Dim currentIndex As Integer
    currentIndex = 0
    Dim tmp As Double
    Do While (numberOfLoops > 0)
        hash = LimitDouble(CDbl(hash) + AscW(Mid$(dataToHash, currentIndex + 1, 1)))
        tmp = shl(AscW(Mid$(dataToHash, currentIndex + 2, 1)), 11) Xor hash
        hash = shl(hash, 16) Xor tmp
        hash = LimitDouble(CDbl(hash) + shr(hash, 11))
        currentIndex = currentIndex + 2
        numberOfLoops = numberOfLoops - 1
    Loop
    If remainingBytes = 1 Then
        hash = LimitDouble(CDbl(hash) + AscW(Mid$(dataToHash, currentIndex + 1, 1)))
        hash = hash Xor shl(hash, 10)
        hash = LimitDouble(CDbl(hash) + shr(hash, 1))
    End If
    '' Final avalanche
    hash = hash Xor shl(hash, 3)
    hash = LimitDouble(CDbl(hash) + shr(hash, 5))
    hash = hash Xor shl(hash, 4)
    hash = LimitDouble(CDbl(hash) + shr(hash, 17))
    hash = hash Xor shl(hash, 25)
    hash = LimitDouble(CDbl(hash) + shr(hash, 6))
    SuperFastHash = hash
End Function

Answer 1

This is quite possibly the most clever VBA code I have ever seen.

There's not much room for improvement here, except a couple naming nitpicks:

• If that's what they stand for, functions shr and shl could afford to be called ShiftRight and ShiftLeft.
• I don't see a reason for shr and shl to not follow the PascalCasing naming convention.
• Given the almost-flawlessly consistent camelCasing convention for locals & parameters, I don't see a reason for Value and Shift parameters to not stick to it. (except perhaps VBA/VB6's stubornness with casing?)

As for string handling, as per this source AscW is faster than Asc so again, good call:

AscW(Mid$(..)) considerations

Don't copy too many characters with Mid$. AscW only examines the first character anyway. AscW(Mid$(s, x, 1)) is the best call. Note that if you call AscW(Mid$(s, x)) without the third parameter, Mid$ executes slowly when s is a long string.

Example of potentially slow code:

For x = 1 To Len(s)
    If AscW(Mid$(s, x)) = ... Then ... 
Next

The above is better written as:

For x = 1 To Len(s)
    If AscW(Mid$(s, x, 1)) = ... Then ... 
Next

Answer 2

Nothing major, just some stylistic comments....

• The Do While loop can be rewritten more clearly as a For i = 0 to dataLength \ 2

• the variable remainingBytes is unnecessary, and if used should be a boolean not an integer.

• tmp can be declared inside the loop as it is never used outside it.

Answer 3

in your SuperFastHash function you are Dimming and setting variables and then using them in a fashion that looks messy to me even though it is in a logical order.

the set up should be separate from the actual logic of the code to make it more clear and readable.

you should Dim all the variables at the beginning of the function and then follow with the logic so that it the flow is clear and understandable

it should look like this:

Public Function SuperFastHash(ByVal dataToHash As String) As Long
    Dim dataLength As Long
    Dim hash As Long
    Dim remainingBytes As Integer
    Dim numberOfLoops As Integer
    Dim currentIndex As Integer
    Dim tmp As Double

    dataLength = Len(dataToHash)
    If (dataLength = 0) Then
        SuperFastHash = 0
    Exit Function
    End If
    hash = dataLength    
    remainingBytes = dataLength Mod 2
    numberOfLoops = dataLength \ 2
    currentIndex = 0

this looks much clearer about what is going on and is a good coding practice

(as I was writing this I thought this was going to be the only answer, but then retailcoder posted an answer)