0

I have a class heirarchy as follows:

public abstract class Issue { // some base properties // these are not included in equality checks for deriving classes // also not performing equality checks on this base class (it is abstract anyway) } public class IssueTypeA : Issue, IEquatable<IssueTypeA> { // some properties specific to this class // these are the two for which equality comparison is performed public string requirement { get; set; } public string preamble { get; set; } public bool Equals(IssueTypeA that) { // determine based on the values of the properties // they must both be the same for equality if ((this.requirement.Equals(that.requirement)) && (this.preamble.Equals(that.preamble))) { return true; } return false; } } public class IssueTypeB : Issue, IEquatable<IssueTypeB> { // some properties specific to this class public bool Equals(IssueTypeB that) { // determine based on the values of the properties } }

There is another class, intended to receive objects in the above heirarchy (except the base class, of course), and do some comparison operations with them:

public class Comparer<T> where T : Issue { // various comparison methods public IEnumerable<T> getReferenceChangedIssues(IEnumerable<T> spreadsheetIssues, IEnumerable<T> downloadedIssues) { foreach (T spreadsheetRecord in T spreadsheetIssues) { foreach (T downloadedIssue in downloadedIssues) { // this is the point of failure // there are cases in which this should be true, but it is not if (spreadsheetRecord.Equals(downloadedIssue)) { // the referenceChanged method works fine by itself // it has been unit tested if (spreadsheetRecord.referenceChanged(downloadedIssue)) yield return spreadsheetRecord; } } } } }

Through unit testing and debugging, it's apparent that the custom Equals methods defined above are not being used correctly by Comparer. Why is this? It is also intended to include methods with some set operations in the future, which will need Equals.

Improve this question
5
  • What if to Compare you're given one IssueTypeA and one IssueTypeB, which Equals method would it call? Commented May 5, 2020 at 4:43
  • @YuriyFaktorovich I have not tried that. I will do, but in normal circumstances two different subtypes of Issue would not be mixed. Perhaps generics are not suitable here. Commented May 5, 2020 at 4:49
  • If you implement IEquatable you should also override Object.Equals and Object.GetHashCode. What methods Comparer include? Does it implement interface IComparer? What problems do you face in the methods of Comparer? Commented May 5, 2020 at 4:52
  • @IliarTurdushev I will add an example of a method included in Comparer. Commented May 5, 2020 at 4:55
  • @Al2110 Now I see that the problem in the definition of the class Comparer. When you call spreadsheetRecord.Equals(downloadedIssue) a method Object.Equals is invoked (instead of IEquatable.Equals), because type T of Comparer does not constrained to implement IEquatable. Change the definition of the Comparer to the next: class Comparer<T> where T : Issue, IEquatable<T>. And also consider overriding Object.Equals and Object.GetHashCode in your Issue classes. Commented May 5, 2020 at 5:08

1 Answer 1

Reset to default
0

When you implement IEquatable, you really need to override bool Equals(object) and GetHashCode. If you don't do that, things will break in weird ways. This was the stripped-down-est thing I could come up with. I think it should solve your issue (but I can't really tell without your comparison code). If folks point out issues, I'll fix up the code:

public abstract class Issue : IEquatable<Issue> { // possibly (not not necessarily) some base properties // if they are included, make them part of the equality comparison and // include them in the HashCode calculations public bool Equals(Issue other) { if (other == null) { return false; } if (this.GetType() != other.GetType()) { return false; } //test local properties and return true/false based on their values return true; } public override bool Equals(object obj) { if (ReferenceEquals(null, obj)) return false; if (ReferenceEquals(this, obj)) return true; if (obj.GetType() != this.GetType()) return false; return Equals((Issue) obj); } } public class IssueTypeA : Issue, IEquatable<IssueTypeA> { public string AName { get; set; } = "My A Name"; public int AValue { get; set; } = 42; public bool Equals(IssueTypeA other) { if (!base.Equals(other)) { return false; } return AName == other.AName && AValue == other.AValue; } public override bool Equals(object obj) { if (ReferenceEquals(null, obj)) return false; if (ReferenceEquals(this, obj)) return true; if (obj.GetType() != this.GetType()) return false; return Equals((IssueTypeA) obj); } //hash-codes need to be consistent once they are calculated - jump through some hoops private int? _hashCodeValue = default; public override int GetHashCode() { if (!_hashCodeValue.HasValue) { _hashCodeValue = $"{AName}{AValue}".GetHashCode(); } return _hashCodeValue.Value; } } public class IssueTypeB : Issue, IEquatable<IssueTypeB> { // some properties specific to this class public string BName { get; set; } = "My B Name"; public int BValue { get; set; } = 84; public bool Equals(IssueTypeB other) { if (!base.Equals(other)) { return false; } //calculate equality based on local properties and return true/false return BName == other.BName && BValue == other.BValue; } public override bool Equals(object obj) { if (ReferenceEquals(null, obj)) return false; if (ReferenceEquals(this, obj)) return true; if (obj.GetType() != this.GetType()) return false; return Equals((IssueTypeB) obj); } //hash-codes need to be consistent once they are calculated - jump through some hoops private int? _hashCodeValue = default; public override int GetHashCode() { if (!_hashCodeValue.HasValue) { _hashCodeValue = $"{BName}{BValue}".GetHashCode(); } return _hashCodeValue.Value; } }

This is a better stab at a solution than the first pass. I check for equality based on properties local to each class. I also provide a GetHashCode implementation that is consistent (in the weird way that I believe GetHashCode requires - but in a way that drive Resharper nuts).

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

I added to the original post, the properties for which the equality comparison for IssueTypeA is based. I see you gave an example in your answer for the case that base properties are used in the comparison. In my case, they are not. Do I still need the equals and get hash code methods in the base class?