Code Review Guidelines: Avoid inheritance for properties
time to read 3 min | 460 words

I recently have gone over some codebase to find something like this:

public interface IAuditable
{
  DateTime UpdatedAt {get;set;}
  string UpdatedBy {get;set;}
  DateTime CreatedAt {get;set;}
  string CreatedBy {get;set;}
}

public interface IEntity
{
  int Id {get;set;}
}

public class Entity : IEntity
{
  public int Id { get;set; }
}

public class AuditableEntity : Entity, IAuditable
{
  public DateTime UpdatedAt {get;set;}
  public string UpdatedBy {get;set;}
  public DateTime CreatedAt {get;set;}
  public string CreatedBy {get;set;}
 
}

public class Visit : AuditableEntity
{
  // stuff
}

I look at code like that, and it is more than a bit painful. It is painful, because this sort of code is badly abusing inheritance.

The problem is that this is mostly intended to save on typing, but with things like automatic properties, there isn’t really much point here. What it does produce is code that seems to be more complicated than it is, because now we have those classes in the middle that does nothing but provide properties for you to use. Worse than that, they take down the only base class slot that you have, and they force you to think in a way that isn’t always natural.

It is just as easy, and much clearer to use:

public interface IAuditable
{
  DateTime UpdatedAt {get;set;}
  string UpdatedBy {get;set;}
  DateTime CreatedAt {get;set;}
  string CreatedBy {get;set;}
}

public interface IEntity
{

}

public class Visit : IAuditable, IEntity
{
  public int Id {get;set;}
  public DateTime UpdatedAt {get;set;}
  public string UpdatedBy {get;set;}
  public DateTime CreatedAt {get;set;}
  public string CreatedBy {get;set;}
 
}

And hey, you can now have an auditable that have a composite key, something that you used to need a completely separate inheritance hierarchy to deal with.

Tweet Share Share 32 comments
Tags: