1

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");
    }
  • what does your `StudentService` look like? does it have any relationship with the `IGeneralService`? If they serve fairly different tasks and not related, they are 2 really different services. So if your controller uses the APIs from both of them, they must have separate instances (as dependencies). Your question is still vague to have an answer. – Hopeless Dec 25 '20 at 09:52
  • Hi @Hopeless thanks for comment. Yes, there are two different service. My StudentService consists of specific operations and business rules. (like GetStudentPhoneNumber or CheckStudentExist etc.) IGeneralService (and GeneralService) doing just general operations. (Like CRUD and GetAll etc.) If I want to do specific operations for classes, I want to use EntityService (in this scenario this is UserService) class. Is this a misuse? – hasaneyldrm Dec 25 '20 at 10:39
  • really I do not get what you mean, you've just introduced 2 new service names (`EntityService` and `UserService`) which are not in your posted code at all. So it's kind of losing the track. – Hopeless Dec 25 '20 at 10:42
  • Let me wrap up the topic. EntityService is actually my StudentService. my entity(Student). I thought it like Entity (Student) Service but I guess I couldn't explain it very well. I'm really sorry for that. Whatever, i just have `GeneralService` and `StudentService` GeneralService doing my general operations (like CRUD) but StudentService has more specific functions (like CheckStudentExist) I hope I could explain my problem well. If there is anything I can't explain, please don't hesitate to ask. Thanks for your help – hasaneyldrm Dec 25 '20 at 11:15
  • I think it's the matter of designing it correctly or not, not the matter of using it correctly or not. Your code uses it just correctly (because your controller may need to use different apis from your `IGeneralService` and `StudentService`). But the point here is if you could somehow combine those services into one (e.g: based on some common abstract service, ...). – Hopeless Dec 25 '20 at 11:24
  • I understand. 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: (I'm updating the question above because it doesn't appear right here) – hasaneyldrm Dec 25 '20 at 11:51

1 Answers1

2

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.

Steve Py
  • 26,149
  • 3
  • 25
  • 43
  • we should not think about the performance seriously at first. Writing an abstract entity service may trade off a bit performance but get back a lot of convenience and reduce a large amount of logically duplicated code. I've used abstract entity service in some projects and it works just fine. It's based on EF, even if you have some custom logic (that cannot be done by the entity service), you can always fall back to using EF directly. Finally in the worst case we need to boost the performance to the best, we can bypass EF and write raw SQL. It's the concept of multi-abstract-level layers. – Hopeless Dec 26 '20 at 18:33
  • 1
    It's not so much about focusing on performance, but rather that the abstraction cripples the capabilities that EF provides. Simply writing abstractions that return back 'TEntity` and `IEnumerable` are terrible standards for building a system. Leveraging projection is one part about building performant queries, but another part about not exposing more about your system than the view requires to function. Outside of complex reporting features I've yet to come across a performance issue that couldn't be improved 10-fold+ if they'd used projection. It's abstraction for the wrong reason. – Steve Py Dec 27 '20 at 05:35
  • as I said, the general purpose EntityService is just a tool that we can apply in many cases (so instead of writing about 30-50 lines of code each time updating, you just need 1-3 lines of code), especially when saving data (I've written my own updating method using reflection to do many manual works). For some scenarios you can always use the EF directly in the way it's designed for. In many small projects (serving just about < 200K users), we don't need to care much about the performance so we can use the general code (each request may add up to just 50-300ms). – Hopeless Dec 27 '20 at 08:13