Is it normal to have a base type for Entities and Value Objects? Please see the code below, which I am currently using in a production application. :

public abstract class Entity
{
    public virtual long Id { get; protected set; }
 
    public override bool Equals(object obj)
    {
        var compareTo = obj as Entity;
 
        if (ReferenceEquals(compareTo, null))
            return false;
 
        if (ReferenceEquals(this, compareTo))
            return true;
 
        if (GetRealType() != compareTo.GetRealType())
            return false;
 
        if (!IsTransient() &amp;&amp; !compareTo.IsTransient() &amp;&amp; Id == compareTo.Id)
            return true;
 
        return false;
    }
 
    public static bool operator ==(Entity a, Entity b)
    {
        if (ReferenceEquals(a, null) &amp;&amp; ReferenceEquals(b, null))
            return true;
 
        if (ReferenceEquals(a, null) || ReferenceEquals(b, null))
            return false;
 
        return a.Equals(b);
    }
 
    public static bool operator !=(Entity a, Entity b)
    {
        return !(a == b);
    }
 
    public override int GetHashCode()
    {
        return (GetRealType().ToString() + Id).GetHashCode();
    }
 
    public virtual bool IsTransient()
    {
        return Id == 0;
    }
 
    public virtual Type GetRealType()
    {
        return NHibernateUtil.GetClass(this);
    }
}


Every Entity class inherits from it.

It is a slight variation of the code here: https://github.com/vkhorikov/DddInAction/blob/master/DddInPractice.Logic/Common/Entity.cs

I am trying to decide whether to remove this base entity type from my application because of the feedback I have received when talking about it.  

Is it normal to have a base Entity type (and base ValueObject) that has a base implementation for comparisons e.g. Object.Equals, IEquality.Equals etc?

It seems logical to me.  Say I have the following entities:

Customer
Supplier
Product
Production
etc


Why is it a bad idea for them all to be compared by ID by default?

I have a separate domain model and data model if that has any bearing.
Review

The following methods are candidates for properties. A property is expected to return fast, without too much calculations and internal flow. I'm assuming NHibernateUtil.GetClass provides a fast lookup.


public virtual bool IsTransient()
{
    return Id == 0;
}

public virtual Type GetRealType()
{
    return NHibernateUtil.GetClass(this);
}



public virtual bool IsTransient =&gt; Id == 0;
public virtual Type RealType =&gt; NHibernateUtil.GetClass(this);




When to use a base class


Since C# allows for single class inheritance, pick a base class that adds sufficient state / operations to the class. Providing base classes like EqualityBase, ComparableBase, DisposableBase, HashCodeBase are bad choices, since they don't provide meaningful context except for a very specific mitigation of boiler-plate code. Which class should Entity implement of the above? You would like to implement all of them, but you can't!
Is Entity provided by the OP a good choice for a base class? If the IsTransient() and GetRealType() methods weren't provided, I would argue against using a base class. You would end up with a class like above (EqualityBase, HashCodeBase mix). Both added methods allow for sufficient context to justify a base class.
Use the power of interface inheritance, because you can implement as many interfaces as you want, and composition, because you can contain and delegate operations to as many contained objects you desire.




Dilemma

Using a base class allows for DRY code. The repetitive boiler-plate code sits in the base class, while the entities are very clean POCO's. However, you force the entities to derive from this base class, limiting the inheritance options for such classes drastically.

Solution

To allow for flexibility in inheritance options for your entities, and mitigating most boiler-plate code, you could provide an interface, a base class and let your entities implement both. The special entities that require to inherit from a different base class, should do so, and implement the interface.

public interface IEntity
{
    long Id { get; }
    bool IsTransient { get; }
    Type RealType { get; }
} 

public abstract class EntityBase : IEntity
{
    public virtual long Id { get; protected set; }
    public virtual bool IsTransient =&gt; Id == 0;
    public virtual Type RealType =&gt; NHibernateUtil.GetClass(this);

    // .. other code (equals, operator overloads, ..)
}


Most entities would implement both the base class and interface:

public class Employee : EntityBase
{
    public string Name { get; set; }
    public string FirstName { get; set; }
    public string BadgeId { get; set; }
}


Allowing for special cases:

public class DocumentElement : TreeNode&lt;DocumentElement&gt;, IEntity
{
    // .. DocumentElement  members
    // .. IEntity members
}


Other layers using the entities don't care about the base class, only the interface.

public interface IRepository&lt;T&gt; where T : IEntity
{
    T Get(int id);
    void Save(T entity);
}