9

I look for an equality between two instances of this struct.

public struct Serie<T>
{
    T[]         X; 
    double[]    Y;

    public Serie(T[] x, double[] y)
    {
        X = x;
        Y = y;
    }

    public override bool Equals(object obj)
    {
        return obj is Serie<T> && this == (Serie<T>)obj;
    } 
    public static bool operator ==(Serie<T> s1, Serie<T> s2)
    {
        return s1.X == s2.X && s1.Y == s2.Y;
    }
    public static bool operator !=(Serie<T> s1, Serie<T> s2)
    {
        return !(s1 == s2);
    }

This doesn't work. What am I missing?

        double[] xa = { 2, 3 };
        double[] ya = { 1, 2 };
        double[] xb = { 2, 3 };
        double[] yb = { 1, 2 };
        Serie<double> A = new Serie<double>(xa, ya);
        Serie<double> B = new Serie<double>(xb, yb);
        Assert.AreEqual(A, B);
Frederic
  • 760
  • 1
  • 5
  • 15
  • Related post - [Comparing boxed value types](https://stackoverflow.com/q/6205029/465053) & [Are value types immutable by definition?](https://stackoverflow.com/q/868411/465053) – RBT Jun 16 '18 at 03:13

6 Answers6

19

You're comparing the array references rather than their contents. ya and yb refer to different arrays. If you want to check the contents of the arrays, you'll have to do so explicitly.

I don't think there's anything built into the framework to do that for you, I'm afraid. Something like this should work though:

public static bool ArraysEqual<T>(T[] first, T[] second)
{
    if (object.ReferenceEquals(first, second))
    {
        return true;
    }
    if (first == null || second == null)
    {
        return false;
    }
    if (first.Length != second.Length)
    {
        return false;
    }
    IEqualityComparer comparer = EqualityComparer<T>.Default;
    for (int i = 0; i < first.Length; i++)
    {
        if (!comparer.Equals(first[i], second[i]))
        {
            return false;
        }
    }
    return true;
}

As an aside, your structs are sort of mutable in that the array contents can be changed after the struct is created. Do you really need this to be a struct?

EDIT: As Nick mentioned in the comments, you should really override GetHashCode as well. Again, you'll need to get the contents from the arrays (and again, this will cause problems if the arrays get changed afterwards). Similar utility method:

public static int GetHashCode<T>(T[] array)
{
    if (array == null)
    {
        return 0;
    }
    IEqualityComparer comparer = EqualityComparer<T>.Default;
    int hash = 17;
    foreach (T item in array)
    {
        hash = hash * 31 + comparer.GetHashCode(item);
    }
    return hash;
}
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • 12
    using LINQ's SequenceEqual() extension method. It can compare array contents. – LBushkin Jan 11 '10 at 14:09
  • @LBushkin: Yes (I should have mentioned that), but it will be somewhat inefficient compared with an implementation which is geared around arrays. – Jon Skeet Jan 11 '10 at 14:10
  • 2
    Possibly, but I tend to prefer clarity over performance, unless there's a measured performance problem. Out of curiosity, what makes you believe `SequenceEquals` would be inefficient? – LBushkin Jan 11 '10 at 14:12
  • 5
    +1 I would add that GetHashCode needs to be overridden when overriding Equals and so you will need to hash the arrays (or use a really bad hash code like always returning a constant). – Nick Guerrera Jan 11 '10 at 14:21
  • 2
    @LBushkin: Unless it's actually *optimized* for arrays somewhere, it won't have the most efficient paths available to it. Arrays can be pretty heavily optimised. I agree about preferring simplicity in general, but this is a useful utility method to write once and then use in lots of places. @Nick: Good point about GetHashCode; will add that to the answer. – Jon Skeet Jan 11 '10 at 14:26
  • 1
    unlike several of the Linq extension methods SequenceEqual has no special casing for lists or arrays (some special casing for lists will work with arrays as they implement IList by magic). It doesn't even do the Count optimization if both implement ICollection or IList – ShuggyCoUk Jan 11 '10 at 14:44
  • 2
    oh and adding my voice to the , this *really* shouldn't be a struct. – ShuggyCoUk Jan 11 '10 at 14:46
  • .NET 4 (released after this post) provides structural comparisons. Array and Tuple types implement IStructuralComparable and IStructuralEquatable. The StructuralComparisons class has a default StructuralComparer and StructuralEqualityComparer that provide you with proper compare, equals and gethashcode implementations for arrays and tuples, although for tuples the default equality semantics are structural anyway. – jvdneste Sep 17 '12 at 08:36
6

I don't think there's anything built into the framework to do that for you, I'm afraid

In 4.0, there is:

StructuralComparisons.StructuralEqualityComparer.Equals(firstArray, secondArray);
Sebastian Ullrich
  • 1,170
  • 6
  • 10
5

You should compare the contents of the Array in your Equality logic ...

Also, it is recommended that you implement IEquatable<T> interface on your struct, as this prevents boxing/unboxing issues in some cases. http://blogs.msdn.com/jaredpar/archive/2009/01/15/if-you-implement-iequatable-t-you-still-must-override-object-s-equals-and-gethashcode.aspx

Frederik Gheysels
  • 56,135
  • 11
  • 101
  • 154
2

The part s1.Y == s2.Y tests if they are 2 references to the same array instance, not if the contents are equal. So despite the title, this question is actually about equality between array(-reference)s.

Some additional advice: Since you are overloading you should design Serie<> as immutable and because of the embedded array I would make it a class instead of a struct.

H H
  • 263,252
  • 30
  • 330
  • 514
2

Calling == performs reference equality on arrays - they don't compare the contents of their elements. This basically means that a1 == a2 will only return true if the exact same instance - which isn't what you want, I think..

You need to modify your operator == to compere the contents of the x array, not it's reference value.

If you're using .NET 3.5 (with link) you can do:

public static bool operator ==(Serie<T> s1, Serie<T> s2)
{
    return ((s1.X == null && s2.X == null) || s1.X.SequenceEquals( s2.X )) 
           && s1.Y == s2.Y;
}

If you need to do deep comparison (beyond references), you can supply SequenceEquals with a custom IEqualityComparer for the type of T.

You probably should also consider implementing the IEquatable<T> interface for your struct. It will help your code work better with LINQ and other parts of the .NET framework that perform object comparisons.

LBushkin
  • 129,300
  • 32
  • 216
  • 265
1

You can create a private accessor for your struct and use CollectionAssert:

[TestMethod()]
public void SerieConstructorTest()
{
    double[] xa = { 2, 3 };
    double[] ya = { 1, 2 };
    double[] xb = { 2, 3 };
    double[] yb = { 1, 2 };
    var A = new Serie_Accessor<double>(xa, ya);
    var B = new Serie_Accessor<double>(xb, yb);
    CollectionAssert.AreEqual(A.X, B.X);
    CollectionAssert.AreEqual(A.Y, B.Y);
}

This code works fine.

References:

bniwredyc
  • 8,649
  • 1
  • 39
  • 52