.net - How to set multiple services from Entity Framework Core on Repository Pattern?

Question

I am trying to build a structure using repository pattern in entity framework core.

I have an generic interface and a service for general operations. They simply do CRUD transactions.

My interface:

  public interface IGeneralService<TEntity> where TEntity : class
{
    void Delete(TEntity entityToDelete);

    void Delete(object id);

    IEnumerable<TEntity> Get(
        Expression<Func<TEntity, bool>> filter = null,
        Func<IQueryable<TEntity>, IOrderedQueryable<TEntity>> orderBy = null,
        string includeProperties = "");

    TEntity GetById(object id);

    void Insert(TEntity entity);

    void Update(TEntity entityToUpdate);

}

My service Layer:

public class GeneralService<TEntity> : IGeneralService<TEntity> where TEntity : class
{
    internal DrivingSchoolContext context;
    internal DbSet<TEntity> dbSet;

    public GeneralService(DrivingSchoolContext context)
    {
        this.context = context;
        this.dbSet = context.Set<TEntity>();
    }

    public virtual IEnumerable<TEntity> Get(
        Expression<Func<TEntity, bool>> filter = null,
        Func<IQueryable<TEntity>, IOrderedQueryable<TEntity>> orderBy = null,
        string includeProperties = "")
    {
        IQueryable<TEntity> query = dbSet;

        if (filter != null)
        {
            query = query.Where(filter);
        }

        if (includeProperties != null)
        {
            foreach (var includeProperty in includeProperties.Split
                (new char[] { ',' }, StringSplitOptions.RemoveEmptyEntries))
            {
                query = query.Include(includeProperty);
            }
        }


        if (orderBy != null)
        {
            return orderBy(query).ToList();
        }
        else
        {
            return query.ToList();
        }
    }
    public virtual TEntity GetById(object id)
    {
        return dbSet.Find(id);
    }
    public virtual void Insert(TEntity entity)
    {
        dbSet.Add(entity);
    }
    public virtual void Delete(object id)
    {
        TEntity entityToDelete = dbSet.Find(id);
        Delete(entityToDelete);
    }
    public virtual void Delete(TEntity entityToDelete)
    {
        if (context.Entry(entityToDelete).State == EntityState.Detached)
        {
            dbSet.Attach(entityToDelete);
        }
        dbSet.Remove(entityToDelete);
    }
    public virtual void Update(TEntity entityToUpdate)
    {
        dbSet.Attach(entityToUpdate);
        context.Entry(entityToUpdate).State = EntityState.Modified;
    }
}

And my Startup.cs

  services.AddScoped(typeof(IGeneralService<>), typeof(GeneralService<>));

If I want to do a get operation in this simple structure, I do this (In my controller)

    [HttpGet]
    public Student GetStudent(int id)
    {
        var student = _generalService.Get(filter: x => x.id == id).FirstOrDefault();
        return student;
    }

I want to place another service layer in front of the controller

when I want to make a insert, it must first comply with my business rules (like phone number , null check etc.) but I couldn't find how to call multiple service (logic) in the "repository pattern" (without creating a new instance)

if i create a new instance like this:

  public static DrivingSchoolContext DrivingSchoolContext;
    private readonly IGeneralService<Student> _generalService;
    private readonly StudentService _studentService;
    public StudentController(DrivingSchoolContext drivingSchoolContext, IGeneralService<Student> generalService)
    {
        DrivingSchoolContext = drivingSchoolContext;
        _generalService = generalService;
        this._studentService = new StudentService(drivingSchoolContext);

    }


    [HttpGet]
    public Student GetStudent(int id)
    {
        var studentFromNewInstance = _studentService.GetStudent(id);
        var student = _generalService.Get(filter: x => x.id == id).FirstOrDefault();
        return student;
    }

that code is working but I have too much services and I don't want to create everythem.

Can I refer to my business rules without creating a new "instance" for each service?

I want to use both of my services as follows. For ex we have a student creation scenario. First of all, I want to go to the StudentService service and apply my business rules. if it passes business rules, I want it to run the Insert method in Generalservice. Basically like this:

 [HttpGet]
    public IActionResult CreateStudent(Student student)
    {

        student.FullName = StudentService.GetFullName(student.firstName + student.lastName);
        _generalService.Insert(student);

        return Ok("Successfully Created Student");
    }

Answer 1

One recommendation would be to avoid using a Generic pattern for Repositories/Services, and don't overly focus too much on abstractions. I know it's tempting and looks like it would be efficient, but it paints you into a corner when it comes to writing performant data access. When it comes to abstractions, my advice is to follow K.I.S.S. and consolidate when and where it is practical after implementing the simplest solution. When it comes to Generics and Inheritance, trying to design for this "up front" is a form of premature optimization. Both Generics and Inheritance should be applied to serve behaviour that is identical. This is absolutely key. Too often I see developers obsess about these early on in a project over behaviour that is merely similar. They are useful for behaviour that is 100% the same, not 95% the same. This either pigeon-hole's you into a less optimal solution, usually constraining performance or your ability to adapt to changing requirements, or it leads to compromises or deviations (i.e. conditional code) which opens the door for bugs, or it adds considerable cost to re-factor for new requirements that don't fit design decisions based on past assumptions/knowledge.

The approach I recommend is to treat repositories like you would a controller. If you have a ManageStudentController, then I'd have a ManageStudentRepository. Low-level business logic can be positioned in the repository based on the requirements of the data store. If you really want to dive deep into separation of concerns you could create a StudentFactory class, but I find the Repository is in a perfect position to take on this responsibility. Ultimately it is up to the View logic to contain business rules to ensure data is valid, the repository serves as the final gate-keeper to the data store. It would hold a CreateStudent method that accepts parameters for all required details to make a minimally complete and valid Student entity, associates it to the DbContext, and returns it. So if details like Name and a Phone Number is required, the CreateStudent method enforces these low-level data rules. It would also accept either FK values or related entities for non-null-able relationships. This is why the repository is a good place for this method since it can validate values (such as checking for uniqueness) and load related entities when given a key. The Controller etc. can then fill in any optional details into the returned student (ideally using DDD mutation methods rather than setters) in a controlled manner before the unit of work commits the changes.

The problem with generic patterns like what you've outlined which is a very common Repository implementation, is that it is very inefficient.

IEnumerable<TEntity> Get(
    Expression<Func<TEntity, bool>> filter = null,
    Func<IQueryable<TEntity>, IOrderedQueryable<TEntity>> orderBy = null,
    string includeProperties = "");

This will always return entities. While you have added expressions for filtering (Where) and sorting (OrderBy/OrderByDescending), includeProperties is relying on magic strings, and there is no support for things like pagination. You also pigeon-hole your data access to synchronous calls unless you also write more logic for await-able flavours as well.

A simpler solution is to leverage IQueryable<TEntity>.

 IQueryable<TEntity> Get();

If you are using a Soft-Delete system (using something like IsActive rather than hard row deletes) then:

IQueryable<TEntity> Get(bool includeInactive = false);

The repository can automatically manage low-level rules like IsActive, Authentication/Authorization applicable to the query. Callers are then completely free to further filter, order, paginate, and project the results however suit them, plus leverage async/await. This gives you ultimate control over the consumption of the data for maximum performance and a very simple abstraction point for unit testing. The typical question becomes "If it's such a thin wrapper over DbSets, why bother with a Repository at all?". The answer to that is two things: A) It is much easier to Mock a Repository and IQueryable<TEntity> than it is to mock a DbContext/DbSet; and B) The Repository still serves as a great place to enforce low-level data rules to ensure entities are "complete enough" as well as Authorization checks, etc.

A common argument against using IQueryable<TEntity> is that people feel it "leaks" EF-isms / knowledge. However, a solution taking expressions and such to try and leverage filtering, sorting, etc. is 100% just as leaky. The filtering & sorting expressions still need to consider what EF will understand, such as not using local functions or unmapped properties etc. To be as flexible as IQueryable<TEntity> you end up re-writing much of IQueryable<TEntity> and still get left short. (like projection /w Select or Automapper's ProjectTo)

By organizing repositories like controllers one argument might be violating DRY (Don't Repeat Yourself) since the process of managing a Student might include repository methods to fetch other related data, so something like GetClasses() to get a list of available classes to associate to a student might also exist in other areas of the application, so other repositories. This is certainly true, but a counter-argument is DRY must respect Single Responsibility Principle. Code should have one reason, and only one reason to change. Whether you write a Repository<Class> or ClassRepository to adhere to DRY, you are violating SRP. It might seem that ClassRepository's only reason to exist is to serve classes, but in fact every Controller/Service that references a ClassRepository imposes a "reason to change" on the repository. Each controller will inevitably want something a little different when it comes to retrieving classes, and these requirements will evolve over time. This introduces complexity, conditional code, or lots of similar but subtle variants of methods, or, to "force" SRP, it requires all code to accept a very stock-standard, minimum-viable-view of the data, forfeiting performance. A ManageStudentRepository on the other hand has only one reason to ever change. To service the ManageStudentController. This also minimizes the # of dependencies in a given controller, so instead of references to maybe a dozen repositories, a controller would have a reference to one or maybe two repositories. (The second being something like a stock-standard LookupRepository that can easily work with the minimum-viable-data) Combined with leveraging IQueryable<TEntity>, any duplication of methods between repositories are very small, simple, and easy to understand Linq expressions.

Lastly, if there is a requirement to have business logic called by multiple entry points, such as an MVC controller and a WebAPI Controller then that would make for a case to treat these controllers as more of an Anaemic service, passing through the parameters to a shared Service class, and then responsible for mapping the response back for the view. Again, this is something I would look to tackle only once that requirement is set in stone, not prematurely. Moving code that interacts with Repositories and projects ViewModels from something like an MVC Controller into a separate service, projecting to a common DTO, and updating the MVC Controller to call that and project the DTOs to ViewModels is a pretty simple process, and chances are it won't need to be done for every action, just a subset of common functionality.

I know this is probably not an "answer" to your question, but hopefully raises some points to consider in your solution.