HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Is it ok to add a static method to the IEqualityComparer implementation?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
addthemethodimplementationiequalitycomparerstatic

Problem

Linq methods like Distinct need IEqualityComparer object. But sometimes I just compares two objects and to create Comparer instance for that is seems redundant. So it would be nice to use static Equals method. But I'm not sure does not it break some guidelines.

public class ByteArrayComparer : EqualityComparer
{
    [DllImport("msvcrt.dll", CallingConvention = CallingConvention.Cdecl)]
    private static extern int memcmp(byte[] b1, byte[] b2, long count);

    //http://stackoverflow.com/a/1445405/2336304
    public override bool Equals(byte[] b1, byte[] b2)
    {
        return AreEqual(b1, b2);
    }

    // http://stackoverflow.com/questions/263400/what-is-the-best-algorithm-for-an-overridden-system-object-gethashcode/263416#263416
    // http://stackoverflow.com/a/7244729/2336304
    public override int GetHashCode(byte[] array)
    {
        unchecked
        {
            return array == null ? 0
                  : array.Aggregate(17, (current, element) => current * 31 + element.GetHashCode());
        }
    }

    public static bool AreEqual(byte[] b1, byte[] b2)
    {
        // Validate buffers are the same length.
        // This also ensures that the count does not exceed the length of either buffer. 
        return b1 != null && b2!= null && b1.Length == b2.Length && memcmp(b1, b2, b1.Length) == 0;
    }
}

Solution

-
The AreEqual() does not do what one would expect.

As you have implemented the method, you say null != null.

-
GetHashCode()

You should consider replacing the tenary with a simple if statement. IMHO you would gain in readability.

public override int GetHashCode(byte[] array)
{
    unchecked
    {
        if (array == null) { return 0; }

        return array.Aggregate(17, (current, element) => current * 31 + element.GetHashCode());
    }
}


-
using a static method here, won't do any harm IMHO.

Code Snippets

public override int GetHashCode(byte[] array)
{
    unchecked
    {
        if (array == null) { return 0; }

        return array.Aggregate(17, (current, element) => current * 31 + element.GetHashCode());
    }
}

Context

StackExchange Code Review Q#73463, answer score: 3

Revisions (0)

No revisions yet.