Best practice: Generic interfaces [closed]

Question

I have made an generic interface like:

public interface IDatabaseElement<T>
{
  IList<T> GetAll();
  T Get(id);
  void Save(T element);
  void Delete(int id);
}

If I have e.g. two elements (Person and Store) which uses only the above methods what is then considered best practice?

A: Making a new interface for each element like:

public interface IPerson : IDatabaseElement<Person> { }
public interface IStore : IDatabaseElement<Store> { }

and then my classes like:

public class Person : IPerson { .... }
public class Store : IStore { .... }

and when instanciating variables:

IPerson person = new Person();
IStore store = new Store();

or B: using the generic interfaces directly like:

public class Person : IDatabaseElement<Person> { .... }
public class Store : IDatabaseElement<Store> { .... }

and when instainciating variables:

IDatabaseElement<Person> person = new Person();
IDatabaseElement<Store> store = new Store();

What is considered best practice?

Answer 1

There is a known design pattern for what you are calling IDatabaseElement<T>; it is called the Repository Pattern. So start by renaming IDatabaseElement<T> to:

public interface IRepository<TEntity> { ... }

Furthermore, since you define the IPerson interface, it seems like you are defining an interface for the Person entity, instead of the repository.

Hiding your entity behind an interface is bad practice because your entities are data objects and interfaces are only needed to abstract behavior.

So instead of calling the interface IPerson, start by calling it IPersonRepository.

On the other hand, if your Person class actually contains data (such as FirstName, LastName, Age, etc), in that case you are mixing responsibilities. Your entities should not know how to retrieve themselves (or other instances!!!) from the database. Retrieving data from the database and holding the data are two different responsibilities and you should separate them (give each responsibility its own class). Your system will very soon become unmaintainable if you violate the Single Responsibility Principle.

Now, making a specific interface for each repository type (such as IPersonRepository) is a bad idea. The main reason for having a generic abstraction is because this makes adding extra behavior (such as crosscutting concerns) much easier, because this allows you to define a single generic decorator, for instance: AuditTrailingRepositoryDecorator<T>. But when you let your person repository implementation inherit from IPersonRepository, you can't wrap it with a generic decorator anymore, simply because all methods you defined on IPersonRepository itself will not be accessible anymore. This also makes it much easier to write unit tests, since in your test suite you will only have to create one single generic fake implementation of IRepository<T>.

If you're not interested in adding crosscutting concerns and the ability to easily test your code base, you can go with the specific (non generic) interfaces such as IPersonRepository and IStoreRepository.