Is it good to have a GUID property in a class to use to override it in GetHashCode?

Question

Is it good to have a GUID property in a class to use override in GetHashCode?

Sort of:

public class Voucher : IComparable<Voucher>, IComparable, IEquatable<Voucher> { private Guid? _guid; private Guid Guid { get { return _guid ?? (_guid = Guid.NewGuid()).GetValueOrDefault(); } } public int Id { get; private set; } public string Number { get; private set; } public DateTime Date { get; private set; } public Voucher(string number, DateTime date) { Number = number; Date = date; } public Voucher(int id, string number, DateTime date) : this(number, date) { Id = id; } public override bool Equals(object obj) { return Equals(obj as Voucher); } public override int GetHashCode() { return Guid.GetHashCode(); } public override string ToString() { return String.Format("[{0}] - [{1:dd/MM/yyyy}]", Number, Date); } #region IComparable<Voucher> Members public int CompareTo(Voucher other) { if (other == null) return -1; if (Date != other.Date) return Date.CompareTo(other.Date); else return Number.CompareTo(other.Number); } #endregion #region IComparable Members public int CompareTo(object obj) { return CompareTo(obj as Voucher); } #endregion #region IEquatable<Voucher> Members public bool Equals(Voucher other) { if (other != null) return (Number == other.Number) && (Date == other.Date); return false; } #endregion } 

Yesterday I found out that in order to override GetHashCode, we should use only immutable members / fields of the class.

For many of my cases, this is just the ID created by the Sql server ID and for new instances that are 0.

So, for many new objects (not stored in the database, so Id is 0), the hash code of the object is the same. Right?

Will the solution use a GUID as an example above? Thanks.

EDIT Class after comments

So, after your comments, I changed it to:

 public class Voucher : IComparable<Voucher>, IComparable, IEquatable<Voucher> { public int Id { get; private set; } public string Number { get; private set; } public DateTime Date { get; private set; } public Voucher(string number, DateTime date) { Number = number; Date = date; } public Voucher(int id, string number, DateTime date) : this(number, date) { Id = id; } public override bool Equals(object obj) { return Equals(obj as Voucher); } public override int GetHashCode() { return Number.GetHashCode() ^ Date.GetHashCode(); } public override string ToString() { return String.Format("[{0}] - [{1:dd/MM/yyyy}]", Number, Date); } #region IComparable<Voucher> Members public int CompareTo(Voucher other) { if (other == null) return -1; if (Date != other.Date) return Date.CompareTo(other.Date); else return Number.CompareTo(other.Number); } #endregion #region IComparable Members public int CompareTo(object obj) { return CompareTo(obj as Voucher); } #endregion #region IEquatable<Voucher> Members public bool Equals(Voucher other) { if (other != null) return (Number == other.Number) && (Date == other.Date); return false; } #endregion } 

I assume this is normal, as the voucher is unchanged.

But if the members of Number and Date were not immutable and could be accessed - changed outside the class? Then what is the solution? Is it enough to just document a class like "Cannot be used in HashCode-dependent lists"?

Answer 1

No, this is not suitable for using a GUID in this way, since it breaks what GetHashCode() should mean, which computes a hash of the contents of the object, if if two objects have the same content, they will have the same hash.

You better implement GetHashCode() , as in this question: SO - What is the best algorithm for GetHashCode? You must consider the entire contents of the object for the hash.

Relevant code from the above link:

 public override int GetHashCode() { unchecked // Overflow is fine, just wrap { int hash = 17; // Suitable nullity checks etc, of course :) hash = hash * 23 + field1.GetHashCode(); hash = hash * 23 + field2.GetHashCode(); hash = hash * 23 + field3.GetHashCode(); return hash; } } 

Answer 2

Using Guid is not necessary, as others have mentioned. But I think that I understand the struggle in terms of comparing unforeseen objects. When comparing objects, we use three levels:

AreSame () = is represented as the same space in memory. We really do not use the method here because 'x == y' does it beautifully.

AreEqual () = equality for us is determined by the same identifier, including 0. If the default id (int), then we call it "empty". So much time we test for new objects using the IsNullOrEmpty () method, which perfectly describes an object that either does not exist, or an object that is fresh and has not yet been preserved.

 //querying distinct persisted vouchers var vouchers = vouchers.Where(w=>!w.IsNullOrEmpty()).Distinct(); 

AreEquivalent () . This is based on the individual properties of the object (for example, a composite key) and is very subjective for the object. For example, if your number / date is a separate voucher, then this will be used for equivalence. You can use an anonymous object or something here to make this clear.

 //(warning: handle nulls appropriately, ideally by creating a better equalitycomparer here.). public override bool AreEquivalent(Voucher voucher){ var propsAsAnonymous = v=>new{v.Number,v.Date}; return propsAsAnonymous(this).Equals(propsAsAnonymous(voucher)); } 

Answer 3

Not.

The contract indicates that two identical objects must have the same hash code.

In your case, if you create two objects with the same content, your hash code will be different for both instances. This violates the contract and potentially the behavior of components that rely on hashCode.

In addition, when you override GetHashCode() , you must also override Equals(object) .

Answer 4

When overriding Equals and GetHashCode rule to follow is that if two instances are equal, then they must have the same hash code. You break this rule by creating unique hash codes for identical instances. This will cause problems with collections, such as Dictionary and HashSet , that rely on GetHashCode , returning the same value for equal elements.

Answer 5

Even if everyone else says that it’s not normal, I would go for “Well, it depends.”

One thing where GUIDs are useful for distributed systems. After all, they are globally unique identifiers, so if you check the equality between the borders of several processes / instances / persistency / etc and pass objects across these borders, I would say that you are doing the right thing.

I regularly use the (sequential) GUID as an identifier for databases. While most database administrators do not like this for performance reasons, this has the advantage that you do not need to check before performing the insertion, thereby preserving the network circuit. Personally, I think this is the best practice for database keys. However, I admit that this is very controversial, and currently it is not considered good practice, I suppose.

However, you are probably not after such things. :-)

If you just want to verify equality in an instance of your program, you should think about what you want to achieve. If you want to group instances by database identifier (f.ex. to check for conflicts), you want to create equality using key members (in this case, probably ID is enough, because it seems to be writing a database to 1 database instance )

If you need unique objects in the application, you can implement the equality yourself (note: the default implementation of Object already works like this). The way to do this is to use RuntimeHelpers.GetHashCode(this) and Object.ReferenceEquals(this, o); . It mainly uses pointers for comparison.

To summarize: what you need is implementation dependent. Usually you want equality because you are f.ex. filling out a Dictionary or HashSet . It also requires that you redefine both Equals and GetHashCode . The implementation you should use is the one that makes the most sense in this context.