Is it ok to have a GUID private property in a class in order to use it in GetHashCode override?

Question

Is it OK to have a GUID private property in a class in order to use it in GetHashCode override?

Something like:

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 have to use only immutable members/fields of the class.

For many of my cases that is only the Id that is produced by identity of the Sql Server and for new instances that is 0.

So for many new objects (not persisted to database thus Id is 0) object hash code is the same. Correct?

Would it be a solution to use GUID like the example above? Thanks.

EDIT Class after comments

So after your comments I've 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 guess that this is OK since Voucher is immutable.

But if members Number and Date were not immutable and could be accessed - altered outside the class? Then what is the solution? Is it enough just to document the class something like "Cannot be used in HashCode depended Lists"?

Answer 1

No, it is not okay to use a GUID in this way as it breaks what GetHashCode() is meant to do, which is calculate a hash of the contents of the object where if two objects have the same content, they will have the same hash.

You should rather implement GetHashCode() like in this question : SO - What is the best algorithm for GetHashCode? You should take the entire contents of the object into account for the hash.

The relevant code from the above link is:

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 a Guid is unnecessary as others have mentioned. But I think I understand the struggle in terms of comparing unpersisted objects. We use three levels when comparing objects:

AreSame() = represented by being the same space in memory. We don't really use a method here because 'x == y' does this nicely.

AreEqual() = equality, for us, is defined by having the same Id, including 0. If the id is default(int) then we refer to it as 'empty'. So much of the time we're testing for new objects with a method 'IsNullOrEmpty()' which nicely describes an object that either doesn't exist, or an object that is fresh and hasn't yet been persisted.

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

AreEquivalent() - This is based on the individual properties of an object (e.g. a composite key) and is very subjective to the object. For instance, if your number/date represented a distinct voucher, then that would be used for equivalency. You can use an anonymous object or something here to keep it 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));
    }