How should I override Equals and GetHashCode for HashSet?

Question

Lets say I Have class:

    public class Ident
    {
        public String Name { get; set; }
        public String SName { get; set; }
    }

and also one more:

    class IdenNode
    {
        public Ident id { get; set; }
        public List<IdenNode> Nodes { get; set; }

        public IdenNode()
        {
            Nodes = new List<IdenNode>();
        }
    }

I want to use HashSet<IdenNode> with mind that two elements of it are same(Equal) if and only if their id.Names are Equal.

So, I'm gonna override Equals and GetHashCode like next:

        public override bool Equals(object obj)
        {
            IdenNode otherNode = obj as IdenNode;

            return otherNode != null && 
                   otherNode.id != null && 
                   id.Name == otherNode.id.Name;
        }

        public override int GetHashCode()
        {
            if (id != null)
                return id.Name.GetHashCode();
            else
                // what should I write here?
        }

Am I think right? What should I place in GetHashCode if so?

UPDATE

Could please tell me is it OK to use == and != in Equals method? Or may be ReferenceEquals or some other?

Also, should I override operators == and != ?

Strings are a terrible thing to use as an identifier, case sensitivity, etc. Are you not able to use something like an int or a GUID (yes its a string but it's guaranteed unique to your system) — Maess
– Maess, Commented Dec 16, 2013 at 14:00
@Maess makes a good point. What if two people have the same name and surname? — Moo-Juice
– Moo-Juice, Commented Dec 16, 2013 at 14:01
@Moo-Juice You are correct, I was thinking about databases, where it is sometimes a string. — Maess
– Maess, Commented Dec 16, 2013 at 14:04
You shouldn't override anything. Create the HashSet with an equality comparer (keep the problem localized). If you do persist, at the very least replace al those set; with private set; — Henk Holterman
– Henk Holterman, Commented Dec 16, 2013 at 14:13

D Stanley · Accepted Answer · 2013-12-16 14:13:55Z

7

If id (or id.Name) is null then it's perfectly fine to return 0. Nullable<T> (like int?) returns 0 for "null" values.

Keep in mind that two objects returning the same value from GetHashCode() does NOT imply equality - it only implies that two objects might be equal. The flip, however, is that two "equal" objects must return the same hash code. Both principles seem to be fulfilled by your definition of Equals and GetHashCode

edited Dec 16, 2013 at 14:13

answered Dec 16, 2013 at 14:01

D Stanley

153k12 gold badges189 silver badges257 bronze badges

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

3 Comments

user2706838 Over a year ago

Got your point about returning 0. Could you tell if I also should override operators "==" and "!="?

D Stanley Over a year ago

@mehow I should clarify that by "nullable types" I mean Nullable<T>. Reference type don't return anything because you can't call GetHashCode on a null reference variable.

user2140173 Over a year ago

@DStanley perfectly valid answer I just was thinking ToString() method on a nullable type. My bad -> +1 to you

Dmitrii Bychenko · Accepted Answer · 2013-12-16 14:13:55Z

5

Beware of nulls! You've got a lot of them. Take care of StackOverflow: try not use == and != within Equals method. Usually, we return 0 as a hash code in case of null, e.g.:

public override bool Equals(object obj) {
  // Often we should compare an instance with itself, 
  // so let's have a special case for it (optimization)
  if (Object.ReferenceEquals(obj, this)) 
    return true;

  IdenNode other = obj as IdenNode;

  // otherNode != null line in your code can cause StackOverflow:
  // "!=" calls "Equals" which in turn calls "!=" etc...
  if (Object.ReferenceEquals(null, other))
    return false;

  // Id can be null
  if (Object.ReferenceEquals(id, other.id))
    return true;
  else if (Object.ReferenceEquals(id, null) || Object.ReferenceEquals(other.id, null))
    return false;

  // Let's be exact when comparing strings:
  // i.e. should we use current locale or not etc
  return String.Equals(id.Name, other.id.Name, StringComparison.Ordinal);
}

public override int GetHashCode() {
  // It's typical to return 0 in case of null
  if (Object.ReferenceEquals(null, id))
    return 0;
  else if (Object.ReferenceEquals(null, id.Name)) // <- Name can be null as well!
    return 0;

  return id.Name.GetHashCode();
}

edited Dec 16, 2013 at 14:13

answered Dec 16, 2013 at 14:07

Dmitrii Bychenko

188k20 gold badges178 silver badges231 bronze badges

3 Comments

S.N Over a year ago

Wise defensive mechanism +1.

user2706838 Over a year ago

Thanks a lot, that is looks like what I want. Could you tell me if I should override "==" and "!=" in my case?

Dmitrii Bychenko Over a year ago

@user2706838: You have no need to override them, but, IMHO, you'd better do it

Eric Lippert · Accepted Answer · 2013-12-16 14:59:37Z

2

What should I place in GetHashCode if so?

Returning zero is fine. Note that defining value equality on a name is a bad idea; I know of at least three other Eric Lipperts in the United States and they're not me. There are literally millions, possibly billions, of people who have a name collision.

Could please tell me is it OK to use "==" and "!=" in Equals method? Or may be ReferenceEquals or some other?

My advice is: when mixing reference and value equality, be very clear. If you intend reference equality, say so.

Also, should I override operators "==" and "!=" ?

Yes. It is confusing to have Equals mean one thing and == mean another.

answered Dec 16, 2013 at 14:59

Eric Lippert

663k185 gold badges1.3k silver badges2.1k bronze badges

Collectives™ on Stack Overflow

How should I override Equals and GetHashCode for HashSet?

3 Answers 3

3 Comments

3 Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

3 Comments

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related