4

I have a class that contains some fields. I need to compare instances of this class by value, so I defined GetHashCode and Equals accordingly. Because the class allows circular references, I need a mechanism to avoid infinite recursion (for a more detailed explanation see Value-equals and circular references: how to resolve infinite recursion?). I solved this problem by modifying my Equals method so that it keeps track of the comparisons done before:

class Foo
{
    public string Name { get; set; }
    public Foo Reference { get; set; }

    public override int GetHashCode() { return Name.GetHashCode(); }

    static HashSet<(Foo,Foo)> checkedPairs
        = new HashSet<(Foo,Foo)>(ValuePairRefEqualityComparer<Foo>.Instance);
        // using an equality comparer that compares corresponding items for reference;
        // implementation here: https://stackoverflow.com/a/46589154/5333340

    public override bool Equals(object obj)
    {
        Foo other = obj as Foo;
        if (other == null)
            return false;

        if !(Name.Equals(other.Name))
            return false;

        if (checkedPairs.Contains((this,other)) || checkedPairs.Contains((other,this)))
            return true;

        checkedPairs.Add((this,other));

        bool refsEqual = Reference.Equals(other.Reference);
        checkedPairs.Clear();
        return refsEqual;
    }
}

Imagine the following code in the main method:

Foo foo1 = new Foo { Name = "foo" };
Foo foo2 = new Foo { Name = "foo" };
foo1.Reference = foo2;
foo2.Reference = foo1;

bool foo_equals_bar = foo1.Equals(foo2);
Console.WriteLine("foo_equals_bar = " + foo_equals_bar);

foo1.Equals(foo2) will store (foo1,foo2) in checkedPairs before it calls foo2.Equals(foo1). Inside foo2.Equals(foo1) it will be noticed that checkedPairs contains (foo1,foo2), and true will be returned. This result is transferred to the equal variable inside the call of foo1.Equals(foo2), then checkedPairs is cleared, and true is finally returned to the main method.

(Without utilizing checkedPairs inside Equals, there would be an infinite recursion jumping between foo1.Equals(foo2) and foo2.Equals(foo1).)

This works allright in my single-threaded, non-concurrent sandbox environment. However, I am only using a static field for checkedPairs because I don't know any other way to transfer the already collected items from one call of Equals to the next inside a call stack.

But with this approach I cannot use a multi-threaded or concurrent environment, where several Equals checks might run in parallel or in a mixed-up order (e.g. due to passing Equals as a delegate and invoking it later on instead of immediately).

Questions:

  1. Will using a thread-static variable work? I am afraid not, because I can imagine that different Equals calls from the same call stack could still be executed on different threads (but I don't know).

  2. Is there a way to make checkedPairs "call stack static"? So that each call stack gets its own copy of checkedPairs? Then for each new call stack, a new (empty) checkedPairs would be created, filled during recursion, and garbage collected after the recursion ends.

Kjara
  • 2,504
  • 15
  • 42
  • 3
    I usually just add a parameter to the Equal() method to pass items. There is no reason Equals has to have one parameter. If you have a class with ICompare make the Equal(object) call MyEqual() with two parameters to do the recursion. – jdweng Oct 05 '17 at 16:11

1 Answers1

2

Thanks jdweng to point me to an easy solution that works for the particular code stated in the question:

Remove the checkedPairs field from the Foo class and replace the Equals method by this code:

public override bool Equals(object obj)
{
    return MyEquals(obj, new HashSet<(Foo,Foo)>(ValuePairRefEqualityComparer<Foo>.Instance));
}

private bool MyEquals(object obj, HashSet<(Foo,Foo)> checkedPairs)
{
    Foo other = obj as Foo;
    if (other == null)
        return false;

    if (!Name.Equals(other.Name))
        return false;

    if (checkedPairs.Contains((this,other)) || checkedPairs.Contains((other,this)))
        return true;

    checkedPairs.Add((this,other));

    return Reference.MyEquals(other.Reference, checkedItems);
}

However, this approach is not going to work in general. Take for example the classes from this question: Value-equals and circular references: how to resolve infinite recursion?, and imagine I defined MyEquals analogously there for both Club and Person. Since MyEquals cannot be called from outside the class (I want it private), there will still be infinite recursion. E.g. when Person.MyEquals is called, it will call FavouriteInstitution.Equals inside, but it should redirect to FavouriteInstitution.MyEquals somehow(with a possibly already filled checkedPairs!). Also, Members.SetEquals(other.Members) will redirect to Person.Equals instead of Person.MyEquals.

Kjara
  • 2,504
  • 15
  • 42
  • 2
    If you do `foo1.Reference = null; foo2.Reference = null;` then `foo1.Equals(foo2);`, `foo1.Equals(foo1);` or `foo2.Equals(foo2);` all will throw null reference exceptions. – Scott Chamberlain Oct 05 '17 at 16:57