How should we really be implenting Equals and GetHashCode for NHibernate entities

Question

There are many questions and answers and articles to this question available but in my opinion there seems to be no real clear/correct answer

For me Ayende has the best generic implementation so far that I've seen : http://ayende.com/blog/2500/generic-entity-equality

....But it is from 2007 ....

Is this the 'best way' to implement these methods especially with regard to NHibernate 3.2 which contains some differences in proxy implementation to earlier versions?

Answer 1

Yes!

You should be overriding Equals and GetHashCode. But, you shouldn't be doing value equality (Name == other.Name && Age == other.Age), you should be doing identity equality!

If you don't, you will most likely run into comparing a proxy of an entity with the real entity and it will be miserable to debug. For example:

public class Blog : EntityBase<Blog>
{
    public virtual string Name { get; set; }

    // This would be configured to lazy-load.
    public virtual IList<Post> Posts { get; protected set; }

    public Blog()
    {
        Posts = new List<Post>();
    }

    public virtual Post AddPost(string title, string body)
    {
        var post = new Post() { Title = title, Body = body, Blog = this };
        Posts.Add(post);
        return post;
    }
}

public class Post : EntityBase<Post>
{
    public virtual string Title { get; set; }
    public virtual string Body { get; set; }
    public virtual Blog Blog { get; set; }

    public virtual bool Remove()
    {
        return Blog.Posts.Remove(this);
    }
}

void Main(string[] args)
{
    var post = session.Load<Post>(postId);

    // If we didn't override Equals, the comparisons for
    // "Blog.Posts.Remove(this)" would all fail because of reference equality. 
    // We'd end up be comparing "this" typeof(Post) with a collection of
    // typeof(PostProxy)!
    post.Remove();

    // If we *didn't* override Equals and *just* did 
    // "post.Blog.Posts.Remove(post)", it'd work because we'd be comparing 
    // typeof(PostProxy) with a collection of typeof(PostProxy) (reference 
    // equality would pass!).
}

Here is an example base class if you're using int as your Id (which could also be abstracted to any identity type):

public abstract class EntityBase<T>
    where T : EntityBase<T>
{
    public virtual int Id { get; protected set; }

    protected bool IsTransient { get { return Id == 0; } }

    public override bool Equals(object obj)
    {
        return EntityEquals(obj as EntityBase<T>);
    }

    protected bool EntityEquals(EntityBase<T> other)
    {
        if (other == null)
        {
            return false;
        }
        // One entity is transient and the other is not.
        else if (IsTransient ^ other.IsTransient)
        {
            return false;
        }
        // Both entities are not saved.
        else if (IsTransient && other.IsTransient)
        {
            return ReferenceEquals(this, other);
        }
        else
        {
            // Compare transient instances.
            return Id == other.Id;
        }
    }

    // The hash code is cached because a requirement of a hash code is that
    // it does not change once calculated. For example, if this entity was
    // added to a hashed collection when transient and then saved, we need
    // the same hash code or else it could get lost because it would no 
    // longer live in the same bin.
    private int? cachedHashCode;

    public override int GetHashCode()
    {
        if (cachedHashCode.HasValue) return cachedHashCode.Value;

        cachedHashCode = IsTransient ? base.GetHashCode() : Id.GetHashCode();
        return cachedHashCode.Value;
    }

    // Maintain equality operator semantics for entities.
    public static bool operator ==(EntityBase<T> x, EntityBase<T> y)
    {
        // By default, == and Equals compares references. In order to 
        // maintain these semantics with entities, we need to compare by 
        // identity value. The Equals(x, y) override is used to guard 
        // against null values; it then calls EntityEquals().
        return Object.Equals(x, y);
    }

    // Maintain inequality operator semantics for entities. 
    public static bool operator !=(EntityBase<T> x, EntityBase<T> y)
    {
        return !(x == y);
    }
}

Answer 2

My personal recommendation is not to implement these methods at all, because doing so forces loading in many cases where it isn't really necessary.

Also, if you don't move entities across sessions, you'll never need this. And even if you do, you can always compare by Id when needed.

Answer 3

I had multiple issues while implementing solution suggested by @TheCloudlessSky.

First, my IDs are not with consistent datatype; some are int, some are Guid and some are string. Also, some are auto generated while other are manually assigned. Other problem may be in future in case I decide to use composite ids. Hence, I cannot put

public virtual int Id { get; protected set; }

in EntityBase base class. I have to define it in respective concrete Entity classes.

Second, as I cannot have Id in base class, it was getting harder to implement bool IsTransient property.

So, I decided to generate Guid per instance and use it to implement GetHashCode and Equals like below:

public abstract class BaseEntity
{
    Guid objectId = Guid.NewGuid();
    public virtual Guid ObjectId { get { return objectId; } }

    public override int GetHashCode()
    {
        return ObjectId.GetHashCode();
    }

    public override bool Equals(object other)
    {
        if(other == null)
            return false;
        if(ObjectId != (other as BaseEntity).ObjectId)
            return false;
        return ReferenceEquals(this, other);
    }
}