0

I have a class Bar that looks like this:

public class Bar : IEquatable<Bar>
{
    public string Stringbar1{ get; set; }
    public string Stringbar2{ get; set; }
    public string Stringbar3{ get; set; }
    public string Stringbar4{ get; set; }

    [JsonConverter(typeof(StringEnumConverter))]
    public EnumFoo1 Enumfoo1{ get; set; }

    public bool IsBar{ get; set; }
    public List<string> StringBars{ get; set; }
    [BsonSerializer(SerializerType = typeof(NullableDateTimeOffsetToUtcSerializer))]
    public DateTimeOffset? FooDate{ get; set; }
    public string Stringbar5{ get; set; }
    [JsonConverter(typeof(StringEnumConverter))]
    public EnumFoo2 EnumFoo2 { get; set; }
    public string StringBar6{ get; set; }
    public int Foo{ get; set; }
 
    public Bar()
    {
        EnumFoo1= EnumFoo1.Unknown;
        EnumFoo2= EnumFoo2.Other;
        StringBars= new List<string>();
    }

 public bool Equals(Bar other)
    {
        if (other == null)
        {
            return false;
        }

        return Stringbar1 == other.Stringbar1&& Stringbar2== other.Stringbar2 && Stringbar3== other.Stringbar3 && Stringbar4== other.Stringbar4 && EnumFoo1== other.EnumFoo1 && IsBar== other.IsBar&&  BothNullOrEquals(StringBars,other.StringBars) && StringBar5==other.StringBar5&& FooDate== other.FooDate && ContractType == other.ContractType && LotNumber == other.LotNumber && Rank == other.Rank;
    }


    public override int GetHashCode()
    {
        var stringbar1Hashcode = Stringbar1== null ? 0 : Stringbar1.GetHashCode();
        var stringbar2HashCode = Stringbar2== null ? 0 : Stringbar2.GetHashCode();
        var stringbar3CodeHashCode = Stringbar3== null ? 0 : Stringbar3.GetHashCode();
        var EnumFoo1HashCode =  EnumFoo1.GetHashCode();
        var Stringbar4HashCode = Stringbar4== null ? 0 : Stringbar4.GetHashCode();
        var isBarHashCode =  IsBar.GetHashCode();
        var strtingBarsHashCode = StringBars== null ? 0 : StringBars.GetHashCode();
        var stringbar5HashCode = Stringbar5== null ? 0 : Stringbar5.GetHashCode();
        var fooDateHashCode = FooDate== null ? 0 : FooDate.GetHashCode();
        var enumFoo2HashCode= EnumFoo2.GetHashCode();
         var stringBar6HasCode = StringBar6== null ? 0 : StringBar6.GetHashCode();
        var fooHashCode= Foo.GetHashCode();
        return stringbar1Hashcode ^ stringbar2HashCode ^ stringbar3CodeHashCode ^ EnumFoo1HashCode ^ Stringbar4HashCode ^ isBarHashCode ^ strtingBarsHashCode ^ stringbar5HashCode ^ fooDateHashCode ^ enumFoo2HashCode ^ stringBar6HasCode  ^ fooHashCode ;
    }
    
    
    public static bool BothNullOrEquals<T>(IEnumerable<T> left, IEnumerable<T> right)
    {
        if (left == null && right == null)
        {
            return true;
        }
        if (left != null && right != null)
        {
            return left.SequenceEqual(right);
        }
        return false;
    }
}

Equals is working as expected but it seems that I am missing something when it comes to GetHashCode cause extension methods like LINQ Distinct are not working as expected. and I know that Distinct uses the GetHashCode method to compare references so any idea what am I doing wrong ?

Mohamad Hammash
  • 237
  • 1
  • 8
  • You've overridden `GetHashCode` but haven't overridden `Equals(Object)`. You're required to override both. I suspect it's that which is causing issues. – Damien_The_Unbeliever Jan 31 '23 at 15:22
  • 2
    What issues do you see with e.g. LINQ Distinct? You say they're not working as expected, so what are your expectations and what do you get instead? – Sergey Kudriavtsev Jan 31 '23 at 15:23
  • You're not actually required to override both. You're required to override `GetHashCode` *if* you override `Equals(Object)`, but not the other way around. – madreflection Jan 31 '23 at 15:23
  • @madreflection - you may say that but the [documentation says otherwise](https://learn.microsoft.com/en-us/dotnet/api/system.object.gethashcode?view=net-7.0#notes-to-inheritors): "Derived classes that override GetHashCode() must also override Equals(Object)". The justification given there does sound a bit garbled but that's what it says. – Damien_The_Unbeliever Jan 31 '23 at 15:25
  • It says that overriding `Equals(Object)` is to guarantee that they have the same hash code, but that's not what overriding `Equals(Object)` does. The statement you're quoting is incorrect. – madreflection Jan 31 '23 at 15:27
  • StringBars.GetHashCode() looks like a bad idea; that could be anything and doesn't match your definition of equality - maybe just skip that one? (along with a `public override bool Equals(object obj) => obj is Bar other && Equals(other);`) – Marc Gravell Jan 31 '23 at 15:27
  • And the documentation for `IEquatable` also says that you *should* override both. – Damien_The_Unbeliever Jan 31 '23 at 15:27
  • *"should"* - That's fine. I agree with that. – madreflection Jan 31 '23 at 15:28
  • Strictly speaking, `public override int GetHashCode() => 0;` is a perfectly valid implementation, if only just to make sure that everything else works. `GetHashCode` is notoriously difficult to implement in a way that gets a broad set of values (making false potential equality less of an issue), which is why the `HashCode` class was created. Using `HashCode.Combine` to generate a hash code is a much better way to implement it. – madreflection Jan 31 '23 at 15:30
  • @madreflection Unfortunately the application is using .netstandard2.0 which does not support this, it was first presented in 2.1. – Mohamad Hammash Jan 31 '23 at 16:25
  • 1
    @MohamadHammash: Got you covered: [Microsoft.Bcl.HashCode](https://www.nuget.org/packages/Microsoft.Bcl.HashCode) – madreflection Jan 31 '23 at 16:28

1 Answers1

1

The problem I can see is in the

var strtingBarsHashCode = StringBars== null ? 0 : StringBars.GetHashCode();

Please, note that for List<string> StringBars HashCode is reference based: hash codes are guaranteed to be equal if StringBars share the same reference only. However, you compare these collections by more relaxing scheme:

BothNullOrEquals(StringBars, other.StringBars)

So, StringBars doesn't have to share the same reference to be equal, they should have equal items only. Note that GetHashCode must not return different codes for equal instances.

Don't make GetHashCode that elaborated; please, note, that GetHashCode should be a fast estimation of Equals; to be so 2 .. 3 selective properties are enough:

public override int GetHashCode() {
  // Let .Net check for null and combine hash codes for you
  return HashCode.Combine(Stringbar1, Stringbar2, Stringbar3);
}

Edit: If you can't use HashCode.Combine, well let it be ^, but, please, don't put many fields and properties into GetHashCode just a few the most selective ones:

public override int GetHashCode() {
  // Let .Net check for null and combine hash codes for you
  return (Stringbar1?.GetHashCode() ?? 0) ^
         (Stringbar2?.GetHashCode() ?? 0) ^
         (Stringbar3?.GetHashCode() ?? 0);
}
Dmitry Bychenko
  • 180,369
  • 20
  • 160
  • 215