0

Assume we have a complex structure containing some other complex types, even possibly containing itself e.g.

public class Box {
    public string Name { get; set; }
    public List<Box> Boxes { get; set; }
    public List<Item> Items { get; set; }
    public SomeOtherComplexType BlaBla { get; set; }
    ...
}

And we need to have a possibility to compare these complex types.

Since overriding object.Equals() will be of a poor perfomance when it comes to generic types I see a lot of usages of IEquatable<>.

The problem is that using IEquatable<> uglifies the code very much mixing the business logic and the technical part of overriding the Equeals() and GetHashCode() methods. And the more bigger our structure is - the larger overridden methods are.

It looks awful and oblige to do the same for all inner types.

public class Box : IEquatable<Box> {
    public string Name { get; set; }
    public List<Box> Boxes { get; set; }
    public List<Item> Items { get; set; }
    public SomeOtherComplexType BlaBla { get; set; }
    
    public bool Equals(Box other)
    {
        if (ReferenceEquals(null, other)) return false;
        if (ReferenceEquals(this, other)) return true;

        return 
            (
                Name == other.Name||
                Name != null &&
                Name.Equals(other.Name)
            ) && 
            (
                Boxes == other.Boxes ||
                Boxes != null &&
                Boxes.SequenceEqual(other.Boxes)
            ) && 
            (
                Items == other.Items ||
                Items != null &&
                Items.SequenceEqual(other.Items)
            ) && 
            (
                BlaBla == other.BlaBla ||
                BlaBla != null &&
                BlaBla.SequenceEqual(other.BlaBla)
            );
    }
    public override int GetHashCode()
    {
        unchecked
        {
            var hashCode = 14;

                if (Name != null)
                hashCode = hashCode * 14 + Name.GetHashCode();
                if (Boxes != null)
                hashCode = hashCode * 14 + Boxes.GetHashCode();
                if (Items != null)
                hashCode = hashCode * 14 + Items.GetHashCode();
                if (BlaBla!= null)
                hashCode = hashCode * 14 + BlaBla.GetHashCode();
            return hashCode;
        }
    }
}

Are there any ways of making it less worse or maybe some other alternatives.

Maybe some ways to hide this implementation and make it as much automated as possible? There are only 4 fields, but what if we are having some complex types with ~30 types in it, with some nested types etc??

Using something like hybrid composite design pattern seems to be even more complex to implement.

fox112358
  • 79
  • 9
  • 4
    If you use [records](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/record) in C# 9, you get all of this code generated for you. – Sweeper May 05 '22 at 16:24
  • 1
    Create a new class implementing `IEqualityComparer<>` and put the comparison logic in there. – ckuri May 05 '22 at 16:24
  • @ckuri well, one common class won't help since we need to know all fields for specific type and reflection isn't something we want to use here. Creating specific EqualityComaprers for each model is just nothing than moving these methods into another class – fox112358 May 05 '22 at 16:29
  • It's unclear what you expect from this question - the aspect of "how do I write a simple equality comparison implementation" seems to be your main issue, but you're focusing earlier on `IEquatable` as an interface. You can have helper methods to make the implementation cleaner, then implement `IEquatable` - but having a different interface that still requires you to implement the same code doesn't address your concerns at all, as far as I can tell. – Jon Skeet May 05 '22 at 16:32
  • @Sweeper, may u tell more, how does it apply to comparing collections nested in classes? should collections be declared as collections of records to make it work? – Bartłomiej Stasiak May 05 '22 at 16:37
  • _"Since overriding object.Equals() will be of a poor perfomance when it comes to generic types"_ can you please elaborate? – Guru Stron May 05 '22 at 16:37
  • @GuruStron https://stackoverflow.com/questions/2734914/whats-the-difference-between-iequatable-and-just-overriding-object-equals – fox112358 May 05 '22 at 16:40
  • Also I pretty much sure that `Boxes.GetHashCode()` and `Items.GetHashCode()` should be removed (otherwise `SequenceEqual` checks make no sense). – Guru Stron May 05 '22 at 16:40
  • @GuruStron "otherwise SequenceEqual checks make no sense" - why is that? – fox112358 May 05 '22 at 16:42
  • @fox112358 but `Box` is not a value type. And _"As noted on Jared Parson's blog though, you must still implement the standard Object.Equals and Object.GetHashcode overrides."_ – Guru Stron May 05 '22 at 16:42
  • 1
    @fox112358 as far as I remember `List<>` does not override `GetHashcode`, so you will get hashcode from the reference, so if you have different lists with the same items in the same order you will end up with different hascodes and will never hit `Equals`. So either remove `Boxes/Items.GetHashcode` or just use `object.ReferenceEquals(boxes, other.Boxes)` – Guru Stron May 05 '22 at 16:44
  • @JonSkeet sorry for not being clear. I was referencing to IEquatable<> since it is most common approach for such use cases from what I see. The question can be shortened to some "lightweight implementation of such functionality". Might be a based on IEquatable approach though. Mixing business logic with technical parts looks very bad, imo – fox112358 May 05 '22 at 16:45
  • 1
    @GuruStron Equals method would be wrapped in `public override bool Equals(object obj)` I just missed to add it here – fox112358 May 05 '22 at 16:48
  • 1
    So I'd suggest editing your question to clarify all of that - the part about "Since overriding object.Equals() will be of a poor perfomance when it comes to generic types" is completely irrelevant. – Jon Skeet May 05 '22 at 16:56
  • "Since overriding object.Equals() will be of a poor perfomance when it comes to generic types I see a lot of usages of IEquatable<>." - what makes you say that? polymorphism works just fine, and in the case of cvalue-types: you get "constrained call" (which avoids boxing) – Marc Gravell May 30 '22 at 12:48

1 Answers1

0

First, IEquatable<T> has no impact on performance for reference types. I would drop all your Equals() logic and be done with it. Unless you really do not want equality by reference.

Then, the only sane solution is to introduce a Guid for the object's identity:

public class Box : IEquatable<Box>
{
    public Box(Guid id)
    {
        Id = id;
    }

    public Guid Id { get; }

    public override bool Equals(object obj) => obj is Box box && Equals(box);
    public bool Equals(Box other) => Id == other.Id;
    public override int GetHashCode() => Id.GetHashCode();
}
l33t
  • 18,692
  • 16
  • 103
  • 180