Value Equality with Bidirectional Association in C#
https://stackoverflow.com/questions/731206
-
06-09-2019 - |
italiano
english
français
española
中国
日本の
العربية
Deutsch
한국어
Português
RussianQuestion
Background
I have two objects which have bidirectional association between them in a C# project I am working on. I need to be able to check for value equality (vs reference equality) for a number of reasons (e.g to use them in collections) and so I am implementing IEquatable and the related functions.
Assumptions
- I am using C# 3.0, .NET 3.5, and Visual Studio 2008 (although it shouldn't matter for equality comparison routine issue).
Constraints
Any solution must:
- Allow for bidirectional association to remain intact while permitting checking for value equality.
- Allow the external uses of the class to call Equals(Object obj) or Equals(T class) from IEquatable and receive the proper behavior (such as in System.Collections.Generic).
Problem
When implementing IEquatable to provide checking for value equality on types with bidirectional association, infinite recursion occurs resulting in a stack overflow.
NOTE: Similarly, using all of the fields of a class in the GetHashCode calculation will result in a similar infinite recursion and resulting stack overflow issue.
Question
How do you check for value equality between two objects which have bidirectional association without resulting in a stack overflow?
Code
NOTE: This code is notional to display the issue, not demonstrate the actual class design I'm using which is running into this problem
using System;
namespace EqualityWithBiDirectionalAssociation
{
public class Person : IEquatable<Person>
{
private string _firstName;
private string _lastName;
private Address _address;
public Person(string firstName, string lastName, Address address)
{
FirstName = firstName;
LastName = lastName;
Address = address;
}
public virtual Address Address
{
get { return _address; }
set { _address = value; }
}
public virtual string FirstName
{
get { return _firstName; }
set { _firstName = value; }
}
public virtual string LastName
{
get { return _lastName; }
set { _lastName = value; }
}
public override bool Equals(object obj)
{
// Use 'as' rather than a cast to get a null rather an exception
// if the object isn't convertible
Person person = obj as Person;
return this.Equals(person);
}
public override int GetHashCode()
{
string composite = FirstName + LastName;
return composite.GetHashCode();
}
#region IEquatable<Person> Members
public virtual bool Equals(Person other)
{
// Per MSDN documentation, x.Equals(null) should return false
if ((object)other == null)
{
return false;
}
return (this.Address.Equals(other.Address)
&& this.FirstName.Equals(other.FirstName)
&& this.LastName.Equals(other.LastName));
}
#endregion
}
public class Address : IEquatable<Address>
{
private string _streetName;
private string _city;
private string _state;
private Person _resident;
public Address(string city, string state, string streetName)
{
City = city;
State = state;
StreetName = streetName;
_resident = null;
}
public virtual string City
{
get { return _city; }
set { _city = value; }
}
public virtual Person Resident
{
get { return _resident; }
set { _resident = value; }
}
public virtual string State
{
get { return _state; }
set { _state = value; }
}
public virtual string StreetName
{
get { return _streetName; }
set { _streetName = value; }
}
public override bool Equals(object obj)
{
// Use 'as' rather than a cast to get a null rather an exception
// if the object isn't convertible
Address address = obj as Address;
return this.Equals(address);
}
public override int GetHashCode()
{
string composite = StreetName + City + State;
return composite.GetHashCode();
}
#region IEquatable<Address> Members
public virtual bool Equals(Address other)
{
// Per MSDN documentation, x.Equals(null) should return false
if ((object)other == null)
{
return false;
}
return (this.City.Equals(other.City)
&& this.State.Equals(other.State)
&& this.StreetName.Equals(other.StreetName)
&& this.Resident.Equals(other.Resident));
}
#endregion
}
public class Program
{
static void Main(string[] args)
{
Address address1 = new Address("seattle", "washington", "Awesome St");
Address address2 = new Address("seattle", "washington", "Awesome St");
Person person1 = new Person("John", "Doe", address1);
address1.Resident = person1;
address2.Resident = person1;
if (address1.Equals(address2)) // <-- Generates a stack overflow!
{
Console.WriteLine("The two addresses are equal");
}
Person person2 = new Person("John", "Doe", address2);
address2.Resident = person2;
if (address1.Equals(address2)) // <-- Generates a stack overflow!
{
Console.WriteLine("The two addresses are equal");
}
Console.Read();
}
}
}
Solution 2
If redesigning the class structure to remove the bidirectional association is possible and reduces the number of problems associated with the implementation, then this is preferred solution.
If this redesign is not possible or introduces equal or greater implementation issues, then one possible solution is to use a specialized Equals method to be called by Equals methods of the classes involved in the bidirectional association. As Mehrdad stated, this shouldn't be too big of a deal since the requirements explicitly ask for this coupling, so you are not introducing one by doing this.
Code
Here is an implementation of this that keeps the specialized methods checking only their own fields. This reduces maintenance problems vs having each class do a per-property comparison of the other class.
using System;
namespace EqualityWithBiDirectionalAssociation
{
public class Person : IEquatable<Person>
{
private string _firstName;
private string _lastName;
private Address _address;
public Person(string firstName, string lastName, Address address)
{
FirstName = firstName;
LastName = lastName;
Address = address;
}
public virtual Address Address
{
get { return _address; }
set { _address = value; }
}
public virtual string FirstName
{
get { return _firstName; }
set { _firstName = value; }
}
public virtual string LastName
{
get { return _lastName; }
set { _lastName = value; }
}
public override bool Equals(object obj)
{
// Use 'as' rather than a cast to get a null rather an exception
// if the object isn't convertible
Person person = obj as Person;
return this.Equals(person);
}
public override int GetHashCode()
{
string composite = FirstName + LastName;
return composite.GetHashCode();
}
internal virtual bool EqualsIgnoringAddress(Person other)
{
// Per MSDN documentation, x.Equals(null) should return false
if ((object)other == null)
{
return false;
}
return ( this.FirstName.Equals(other.FirstName)
&& this.LastName.Equals(other.LastName));
}
#region IEquatable<Person> Members
public virtual bool Equals(Person other)
{
// Per MSDN documentation, x.Equals(null) should return false
if ((object)other == null)
{
return false;
}
return (this.Address.EqualsIgnoringPerson(other.Address) // Don't have Address check it's person
&& this.FirstName.Equals(other.FirstName)
&& this.LastName.Equals(other.LastName));
}
#endregion
}
public class Address : IEquatable<Address>
{
private string _streetName;
private string _city;
private string _state;
private Person _resident;
public Address(string city, string state, string streetName)
{
City = city;
State = state;
StreetName = streetName;
_resident = null;
}
public virtual string City
{
get { return _city; }
set { _city = value; }
}
public virtual Person Resident
{
get { return _resident; }
set { _resident = value; }
}
public virtual string State
{
get { return _state; }
set { _state = value; }
}
public virtual string StreetName
{
get { return _streetName; }
set { _streetName = value; }
}
public override bool Equals(object obj)
{
// Use 'as' rather than a cast to get a null rather an exception
// if the object isn't convertible
Address address = obj as Address;
return this.Equals(address);
}
public override int GetHashCode()
{
string composite = StreetName + City + State;
return composite.GetHashCode();
}
internal virtual bool EqualsIgnoringPerson(Address other)
{
// Per MSDN documentation, x.Equals(null) should return false
if ((object)other == null)
{
return false;
}
return (this.City.Equals(other.City)
&& this.State.Equals(other.State)
&& this.StreetName.Equals(other.StreetName));
}
#region IEquatable<Address> Members
public virtual bool Equals(Address other)
{
// Per MSDN documentation, x.Equals(null) should return false
if ((object)other == null)
{
return false;
}
return (this.City.Equals(other.City)
&& this.State.Equals(other.State)
&& this.StreetName.Equals(other.StreetName)
&& this.Resident.EqualsIgnoringAddress(other.Resident));
}
#endregion
}
public class Program
{
static void Main(string[] args)
{
Address address1 = new Address("seattle", "washington", "Awesome St");
Address address2 = new Address("seattle", "washington", "Awesome St");
Person person1 = new Person("John", "Doe", address1);
address1.Resident = person1;
address2.Resident = person1;
if (address1.Equals(address2)) // <-- No stack overflow!
{
Console.WriteLine("The two addresses are equal");
}
Person person2 = new Person("John", "Doe", address2);
address2.Resident = person2;
if (address1.Equals(address2)) // <-- No a stack overflow!
{
Console.WriteLine("The two addresses are equal");
}
Console.Read();
}
}
}
Output
The two addresses are equal.
The two addresses are equal.
OTHER TIPS
You are coupling classes too tightly and mixing values and references. You should either consider checking reference equality for one of the classes or make them aware of each other (by providing an internal specialized Equals method for the specific class or manually checking value equality of the other class). This shouldn't be a big deal since your requirements explicitly ask for this coupling so you are not introducing one by doing this.
I think the best solution here is to break up the Address class into two parts
- Core Address Information (say Address)
1 + Person Information (say OccupiedAddress)
Then it would be fairly simple in the Person class to compare the core address information without creating a SO.
Yes this does create a bit of coupling in your code because Person will now have a bit of inner knowledge about how OccupiedAddress works. But these classes already have tight coupling so really you've made the problem no worse.
The ideal solution would be to completely decouple these classes.
public override bool Equals(object obj){
// Use 'as' rather than a cast to get a null rather an exception
// if the object isn't convertible .
Person person = obj as Person;
return this.Equals(person); // wrong
this.FirstName.Equals(person.FirstName)
this.LastName.Equals(person.LastName)
// and so on
}
I would say, don't call 'this.Resident.Equals(other.Resident));'
More than one person can live at an address so checking the resident is wrong. An address is an address regardless of who is living there!
Without knowing your domain, it's hard to confirm this, but defining equality between two parents based on their childrens relationship back to them seems a bit smelly!
Do your parents really have no way of identifying themselves without checking their children? Do your children really have a unique ID in, and of themselves, or are they really defined by their parent and its relationship to their siblings?
If you have some kind of unique hierarchy, that is unique only because of its relationships, I would suggest your equality tests should recurse to the root, and make an equality check based on the tree relationship itself.
