Let me start with stating that inheritance is not bad, in fact frankly I’m in love with it and use it a lot, but using it in the wrong places is like deciding to ride a motorbike in heavy snow with a warning of a tsunami triggered by an earthquake which is just refusing to stop.

The context

A long time ago in a galaxy far far away, I started work on a software (a web app) which was started years before that and had a requirement for it to be multi-lingual, it was a branch off (or add-on) of another successful product (legacy windows app) of the company and somebody somewhere decided that it’s much easier to use the same database and separate stuff out by schema, I’m not belittling this decision to be clear, I don’t know the circumstances of the time and it isn’t really wasn’t that hard to separate it out at the end, but that’s not what this story is about anyway.

This was the requirement for every domain entity (after stripping out a thousand other irrelevant details):

1. Every entity has a “Name” field
2. Every entity must support any number of languages for the “Name” field
3. Every entity must be auditable, it must have therefore:
   1. CreatedBy
   2. CreatedOn
   3. ModifiedBy
   4. ModifiedOn
4. Every entity must have an IsActive column to indicate whether it shows up in the UI
5. Every entity must have a “Code” field indicating a business friendly code which users looking at in any language could relate to

This is the data model that was derived as a result:

    public abstract class Entity
    {
        public int Id { get; set; }
        public int CreatedById { get; set; }
        public DateTime CreatedOn { get; set; }
        public int ModifiedById { get; set; }
        public DateTime ModifiedOn { get; set; }
    }

    public abstract class CodedEntity : Entity
    {
        public bool Active { get; set; }
        public string Code { get; set; }
    }

    public abstract class Translation : Entity
    {
        public int CultureID { get; set; }

        public int TranslationOfID { get; set; } //the Id of entity that the translation is for

        public virtual string Name { get; set; }
    }

    public abstract class MultilingualEntity : CodedEntity
        where TTranslationEntity : Translation, new()
    {
        public string Name { get; set; }
        public virtual Collection Translations { get; set; }
    }

    public class AbcEntityTranslation : Translation
    {

    }

    public class AbcEntity : MultilingualEntity
    {

    }

The above design led to some cool generic stuff which could do something like this:

public class BlahContext : DbContext
    {
        public DbSet AbcEntities { get; set; }
        public DbSet AbcEntityTranslations { get; set; }
        //some other entities
    }

    public abstract class EntityFrameworkRepositoryBase
        where TTranslation : Translation, new()
        where TEntity : MultilingualEntity
    {
        private readonly BlahContext _context;
        protected EntityFrameworkRepositoryBase(BlahContext context)
        {
            _context = context;
        }

        public async Task GetAsync(Expression<Func<TEntity, bool>> predicate)
        {
            return await _context.Set().Where(predicate).ToListAsync();
        }

        public async Task<int> DeleteByIdAsync(int id)
        {
            var set = _context.Set();
            var entity = await set.FirstAsync(x => x.Id == id);

            set.Remove(entity);

            return await _context.SaveChangesAsync();
        }
    }

Useful, right? This is what inheritance is all about, right?

It’s two years later and we now decide to separate the Siamese applications by separating out the databases and syncing data between them via API’s, to proceed further we first decided which entities would be owned by which application so that the other uses it as read-only data.

So now.. half of those entities would:

1. Have no created by, modified by etc, as these are synced by the system (a fixed user’s id is redundant)

2. Translations recieved by these entities are in separate dedicated tables as the other tables have UI functionality to edit those translations, this data is no longer owned by the application and is read-only.

3. The code field now has different requirements (length) for each of these.

4. The active flag no longer applies to half of the read-only set (functionality change).

Hmmm… But we now have 23 entities owned by the other app referenced by 17 other classes consumed by 46 API endpoints (not kidding) consumed by I don’t know how many pages via angular services which expects all these entities to have that base class (and to have that class have the other base class) with the same properties ….well…fuck!

We achieved by using EF hacks, ignoring it in a lot of places, putting default values in other columns, basically a lot of duct tape solutions as otherwise it would have taken months to do it right with a lot of testing and nobody was willing to approve that.

What we should have done!

Enter SOLID principles! Now I’m not going to rant about what these are, people better than me have already written extensively on this topic, I recommend going through the Wikipedia article first and then taking time out to read this by Uncle Bob.

Now, let’s apply SOLID to the above code and get the entities to look a little different (while keeping in mind that I typed these in 5 minutes, not meant for copy-paste :) ):

public interface IKeyedEntity
    {
        int Id { get; set; }
    }
    public interface IAuditable
    {
        int CreatedById { get; set; }
        DateTime CreatedOn { get; set; }
        int ModifiedById { get; set; }
        DateTime ModifiedOn { get; set; }
    }
    public interface ICoded //I hate this name
    {
        string Code { get; set; }
    }

    public interface IArchivable
    {
        bool IsArchived { get; set; } //Active before, I just like this better, no need for setting default to true.
    }

    public interface ITranslation
    {
        string Value { get; set; } //why restrict to "Name"?
        int CultureId { get; set; }
    }

    public interface ITranslatable
    {
        ICollection Translations { get; set; }
    }

    public class AbcEntity : IKeyedEntity, IAuditable, ICoded, IArchivable, ITranslatable
    {
        public int Id { get; set; }
        public int CreatedById { get; set; }
        public DateTime CreatedOn { get; set; }
        public int ModifiedById { get; set; }
        public DateTime ModifiedOn { get; set; }
        public string Code { get; set; }
        public bool IsArchived { get; set; }
        public ICollection Translations { get; set; }
    }

    public class AbcEntityTranslation : IKeyedEntity, ITranslation //maybe I would have even added IAuditable here
    {
        public int Id { get; set; }
        public string Value { get; set; }
        public int CultureId { get; set; }
    }

    public class BlahContext : DbContext
    {
        public DbSet AbcEntities { get; set; }
        public DbSet AbcEntityTranslations { get; set; }
        //some other entities
    }

    public interface IEntityDeletionProvider
    {
        Task<int> DeleteByIdAsync(int id) where TEntity : class, IKeyedEntity;
    }

    public class EntityDeletionProvider : IEntityDeletionProvider
    {
        private readonly BlahContext _context;

        public EntityDeletionProvider(BlahContext context)
        {
            _context = context;
        }

        public async Task<int> DeleteByIdAsync(int id)
            where TEntity : class, IKeyedEntity
        {
            var set = _context.Set();
            var entity = await set.FirstAsync(x => x.Id == id);

            set.Remove(entity);

            return await _context.SaveChangesAsync();
        }
    }

    //and another to list entities, fill in properties for auditable entities and so on.

I can now remove the functionality I want in an entity and only that functionality, whatever UI code breaks now needs to break anyway as it’s then definetly trying to write something which is supposed to be read-only (or whatever functionality I don’t want it to have) and things are much simpler, faster to change. The classes which were doing mammoth tasks expecting a heirarchy of inheritance are now broken down into smaller ones which do specific things while still keeping the usefulness of having central code for one functionality.

Don’t judge if I missed something somewhere, the key takeaway is:

FOLLOW SOLID!

and also:

Choose composition over inheritance!

Let me know by commenting below if you disagree or want to add your own story! Also, subscribe to email alerts in case you are interested in such posts in the future.