8
\$\begingroup\$

Are there any improvements to these two classes which implement FNV-1a for hashing? Particularly, I'm looking for any reuse opportunities (possibly via a common abstract superclass derived from HashAlgorithm?).

GitHub

public sealed class Fnv1a32 : HashAlgorithm
{
    private const uint FnvPrime = unchecked(16777619);

    private const uint FnvOffsetBasis = unchecked(2166136261);

    private uint hash;

    public Fnv1a32()
    {
        this.Reset();
    }

    public override void Initialize()
    {
        this.Reset();
    }

    protected override void HashCore(byte[] array, int ibStart, int cbSize)
    {
        for (var i = ibStart; i < cbSize; i++)
        {
            unchecked
            {
                this.hash ^= array[i];
                this.hash *= FnvPrime;
            }
        }
    }

    protected override byte[] HashFinal()
    {
        return BitConverter.GetBytes(this.hash);
    }

    private void Reset()
    {
        this.hash = FnvOffsetBasis;
    }
}

and

public sealed class Fnv1a64 : HashAlgorithm
{
    private const ulong FnvPrime = unchecked(1099511628211);

    private const ulong FnvOffsetBasis = unchecked(14695981039346656037);

    private ulong hash;

    public Fnv1a64()
    {
        this.Reset();
    }

    public override void Initialize()
    {
        this.Reset();
    }

    protected override void HashCore(byte[] array, int ibStart, int cbSize)
    {
        for (var i = ibStart; i < cbSize; i++)
        {
            unchecked
            {
                this.hash ^= array[i];
                this.hash *= FnvPrime;
            }
        }
    }

    protected override byte[] HashFinal()
    {
        return BitConverter.GetBytes(this.hash);
    }

    private void Reset()
    {
        this.hash = FnvOffsetBasis;
    }
}
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

If you were being completely asinine you could move the Initialize method to a new common ancestor class. The amount of code you would write for that common class would exceed the amount of code you would save from duplication though, in other words, not worth it. Alternatively, you can move the Reset logic in to the Initialize method, and remove Reset entirely, and just call Initialize in the constructor.

I think two factors come in to play here:

  1. The basic Object-model of the Hashing systems is pretty well structured. The common-code is already abstracted to higher inheritance levels, so the amount of required duplication is really small (Kudos to the .net library team).
  2. By design, the FNV1a algorithm is simple and fast. The only difference between the 64 and 32 bit versions is the value and type of the primes and initializers. Because these values are different the core algorithm (due to the low-level bitwise operations) need to be different.

In other words, your code is as good as it gets. Good job. Now, when you get around to the 128, 256, 512, and 1024-bit versions of the FNV1a algorithm, you may find some other opportunities for logic sharing. I suspect that the loops required using a common int-based infrastructure will make things possible.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.