Overriding GetHashCode with different properties

Question

I have this object:

public class Foo  {
    public string MyOwnId { get; set; }
    public Guid FooGuid { get; } = Guid.NewGuid();
}

I would like Equals() to only care about those with MyOwnId, otherwise they are never equal. When a Foo has a MyOwnId I try to use it, otherwise I want to use FooGuid.

Since FooGuid probably never will be the same, I did something like this:

public bool Equals(Foo foo) {
        if (foo== null) return false;
        return MyOwnId.Equals(foo.MyOwnId);
    }

    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((Foo)obj);
    }

    public override int GetHashCode() {
        int hash = 13;
        hash = (hash*7) + (!string.IsNullOrEmpty(MyOwnId) ? MyOwnId.GetHashCode() : FooGuid.GetHashCode());
        return hash;
    }

Is this a proper way to do what I want? Or do I also need change my Equals method so it looks the same like my GetHashCode? For e.g:

public bool Equals(Foo foo) {
        if (foo == null) return false;
        if (string.IsNullOrEmpty(MyOwnId) || string.IsNullOrEmpty(foo.MyOwnId)) return false;
        return MyOwnId.Equals(foo.MyOwnId);
    }

If MyOwnId == null then return MyOwnId.Equals(foo.MyOwnId); will throw an exception. Change it to String.Equals(MyOwnId, foo.MyOwnId) — Dmitrii Bychenko
– Dmitrii Bychenko, Commented Jan 28, 2016 at 14:11
@DmitryBychenko Ok, but how about my other example of Equals? That takes care about null values — MrProgram
– MrProgram, Commented Jan 28, 2016 at 14:14
If two instances (say, A and B) are equal according to Equals they must have the same hash code. However, GetHashCode() depends on FooGuid and Equals doesn't so the rule is violated. — Dmitrii Bychenko
– Dmitrii Bychenko, Commented Jan 28, 2016 at 14:17

Dmitrii Bychenko · Accepted Answer · 2016-01-28 14:57:15Z

Well, let's see. Your implementation of Equals and GetHashCode is erroneous.

Both Equals and GetHashCode must never throw an exception; the counter example is

  Foo A = new Foo();
  Foo B = new Foo() {
    MyOwnId = "bla-bla-bla",
  };

  // Throws an exception
  if (A.Equals(B)) {}

If two instances are equal via Equals these instances must have the same hash code; the counter example is

  Foo A = new Foo() {
    MyOwnId = "",
  };

  Foo B = new Foo() {
    MyOwnId = "",
  };

  if (A.Equals(B)) {
    // Hashcodes must be equal and they are not
    Console.Write(String.Format("{0} != {1}", A.GetHashCode(), B.GetHashCode()));
  }

Possible (simplest) implementation

// since you've declared Equals(Foo other) let others know via interface implementation
public class Foo: IEquatable<Foo> { 
  public string MyOwnId { get; set; }
  public Guid FooGuid { get; } = Guid.NewGuid();

  public bool Equals(Foo other) {
    if (Object.ReferenceEquals(this, other))
      return true;
    else if (Object.ReferenceEquals(null, other))
      return false;
    else
      return String.Equals(MyOwnId, other.MyOwnId);
  }

  public override bool Equals(object obj) {
    return Equals(obj as Foo); // do not repeat youself: you've got Equals already
  }

  public override int GetHashCode() {
    // String.GetHashCode is good enough, do not re-invent a wheel
    return null == MyOwnId ? 0 : MyOwnId.GetHashCode(); 
  }
}

Jon Hanna · Accepted Answer · 2016-01-28 14:35:40Z

Or do I also need change my Equals method so it looks the same like my GetHashCode?

You change your Equals to match how you want equality to be resolved. You've done this.

You change your GetHashCode() to key on the same information. In this case:

public override int GetHashCode()
{
  return MyOwnId == null ? 0 : MyOwnId.GetHashCode();
}

Incidentally, your Equals(object) is a bit overly-complicated. I would use:

public override bool Equals(object obj)
{
  return Equals(obj as Foo);
}

This passes handling the case of obj being null to the specific Equals() (which has to handle it too), deals with obj being something that isn't a Foo by passing that Equals() a null (so false anyway) and passes the handling of the case of obj being something derived from Foo to the more specific too (which again, has to handle that).

The short-cut on ReferenceEquals isn't worth doing here as there's only one field being compared, and its comparison will have the same ReferenceEquals shortcut. You don't though handle foo being a derived type in the specialised Foo. If Foo isn't sealed you should include that:

public bool Equals(Foo foo)
{
  return (object)foo != null &&
    foo.GetType() == GetType() &&
    MyOwnId.Equals(foo.MyOwnId);
}

If Foo is sealed then that GetType() comparison should be omitted.

If the logic of the Equals() was more complicated than this then the likes of:

public bool Equals(Foo foo)
{
  if ((object)foo == (object)this)
    return true;
  return (object)foo != null &&
    foo.GetType() == GetType() &&
    // Some more complicated logic here.
}

Would indeed be beneficial, but again it should be in the specific overload not the general override.

(Doing a reference-equality check is more beneficial again in == overloads, since they have to consider the possibility of both operands being null so they might as well consider that of them both being the same which implicitly includes that case).

dustinmoris · Accepted Answer · 2016-01-28 14:21:21Z

A hash function must have the following properties:

If two objects compare as equal, the GetHashCode method for each object must return the same value. However, if two objects do not compare as equal, the GetHashCode methods for the two objects do not have to return different values.
The GetHashCode method for an object must consistently return the same hash code as long as there is no modification to the object state that determines the return value of the object's Equals method. Note that this is true only for the current execution of an application, and that a different hash code can be returned if the application is run again.
For the best performance, a hash function should generate an even distribution for all input, including input that is heavily clustered. An implication is that small modifications to object state should result in large modifications to the resulting hash code for best hash table performance.
Hash functions should be inexpensive to compute.
The GetHashCode method should not throw exceptions.

See https://msdn.microsoft.com/en-us/library/system.object.gethashcode(v=vs.110).aspx

Collectives™ on Stack Overflow

Overriding GetHashCode with different properties

3 Answers 3

Comments

Comments

Comments

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Related