2

I am using a System.Collections.Generic, which contains instances of a class I wrote.

I have read that the collections .Contains method uses object.Equals(), or an implementation of the Equals() method from the IEquatable interface.

I have overridden the object method, as well as implemented from the interface. However, Queue.Contains(instance) is always returning false. What am I doing wrong?

For instance...

class Foo : IEquatable<Foo>
{
    ...
    int fooField1;
    ...

    public override bool Equals(object obj)
    {
         Foo other = obj as Foo;
         bool isEqual = false;

         if (other.fooField1 == this.fooField1)
         { 
             isEqual = true;
         }

         return isEqual;         
    }

    public bool Equals(Foo other)
    {
         bool isEqual = false;

         if (other.fooField1 == this.fooField1)
         { 
             isEqual = true;
         }

         return isEqual;         
    }

}
...

void SomeFunction()
{
    Queue<Foo> Q = new Queue<Foo>();
    Foo fooInstance1 = new Foo();
    Foo fooInstance2 = new Foo();

    fooInstance1.fooField1 = 5;
    fooInstance2.fooField1 = 5;

    Q.Enqueue(fooInstanec1);
    if(Q.Contains(fooInstance2) == false)
    {
         Q.Enqueue(fooInstance2);
    }
}

fooInstance2 is always added to the queue. In fact, when I run this on the debugger, the implementations of Equals are never reached.

What am I doing wrong?

John Saunders
  • 160,644
  • 26
  • 247
  • 397
peace_within_reach
  • 237
  • 2
  • 4
  • 14
  • 3
    This code doesn't compile. Queue does not contain an `Add` method (did you mean `Enqueue`?) and also you are setting `fooInstance2.fooField2` which doesn't exist (I assume you meant `fooField1`, which isn't public which is a problem if `SomeFunction` is not inside `Foo`). – Samuel Jan 29 '11 at 03:14
  • Have you copied and pasted your code *exactly* from your IDE? Because when I run this code on the debugger, nothing happens (except a bunch of compiler errors). It's hard to truly diagnose your problem if we're not working with the same code. – Cody Gray - on strike Jan 29 '11 at 03:20
  • I'm sorry -- This is a much-simplified version of the code I'm working on. It's just what I typed into the StackOverflow message box. I tried to correct the obvious typos and compile errors. – peace_within_reach Jan 29 '11 at 03:22
  • @Only Solitaire: If you're not showing us the real code how can we help you? I appreciate you're showing us a simple example, but can you try to make your code simpler whilst still reproducing the problem? – Samuel Jan 29 '11 at 03:26
  • I was hoping that there's a problem with the approach I'm using which would be clear to an expert without needing to run any code. But I will post a working version instead – peace_within_reach Jan 29 '11 at 03:27
  • Well, this is truly bizzare. I have implemented a similar test function similar to Gishu's correction below using the actual program classes. Amazingly, this works correctly and the breakpoint inside the definition of Equals() is hit. However, when the real program code calls Contains(), it doesn't work. I have no idea why. – peace_within_reach Jan 29 '11 at 03:53
  • @Only - check for differences between the two - Are you using a custom collection class ? Without seeing the actual code, I'm afraid I can't offer any more pointers. – Gishu Jan 30 '11 at 04:01

4 Answers4

4

Your sample code works as expected once the initial compile errors are sorted out. Not that it is relevant to the posed problem, Do read up on overriding Equals (you need to override GetHashCode too and check for error cases like null / type-mismatch).

class Foo : IEquatable<Foo>
    {
        private int _fooField1;
        public Foo(int value)
        {
            _fooField1 = value;
        }

        public override bool Equals(object obj)
        {
            return Equals(obj as Foo);
        }

        public bool Equals(Foo other)
        {
            return (other._fooField1 == this._fooField1);
        }
    }



    class Program
    {
        static void SomeFunction()
        {
            var Q = new Queue<Foo>();
            Foo fooInstance1 = new Foo(5);
            Foo fooInstance2 = new Foo(5);

            Q.Enqueue(fooInstance1);
            if (!Q.Contains(fooInstance2))
            {
                Q.Enqueue(fooInstance2);
            }
            else
            {
                Console.Out.WriteLine("Q already contains an equivalent instance ");
            }
        }

        static void Main(string[] args)
        {
            SomeFunction();
        }
    }
Gishu
  • 134,492
  • 47
  • 225
  • 308
0

You need GetHashCode() method overridden as well in your class Foo.

d-live
  • 7,926
  • 3
  • 22
  • 16
0
fooInstance1.fooField1 = 5;
fooInstance1.fooField2 = 5;

You updated fooInstance1 twice there. Second line should say fooInstance2.fooField1 = 5;.

Once that's fixed, Q.Contains returns True as expected.


Other than that, you don't necessarily need to implement IEquatable. Every object has an overridable Equals method. You can simply overwrite that. Be careful when you implement your own comparison method. Your implementation shown in this sample is very open to NullReference exceptions. Something like this would be better:
public override bool Equals(object obj)
{
    if(obj == null)
        return false;

    Foo other = obj as Foo;
    if(other == null)
        return false;

    return fooField1 == other.fooField1;
}

As others have mentioned, if you go this route and override Equals, you should override GetHashCode, too. There are a few other things you should consider. See this MSDN page for details.

Adam Lear
  • 38,111
  • 12
  • 81
  • 101
0

Why is it important to override GetHashCode when Equals method is overriden in C#?

Community
  • 1
  • 1
New Guy
  • 8,606
  • 1
  • 15
  • 12
  • If the value is stored in a Dictionary (for example), and then you try to retrieve it from the dictionary, the dictionary will not search every key it has by calling the Equals method (this would be too slow). Instead it will call GetHashCode on the key you are trying to retrieve. It will use that hash value to index into a small subset of the members of the dictionary and then just test those for equality. – holmes Jan 29 '11 at 07:32