none
design pattern question (repository pattern - service layer) RRS feed

  • Question

  • Hi,

    I am looking for the best practice to implement the following scenario, I am using a repository pattern where I created multiple services and I am calling one service per a controller. My question is which is the best practice to use a service inside another service? I have many options like calling Service A inside Service B by injecting IService A in Service B constructor! or I can use inheritance like Service B can extend Service A then I can use all Service A methods inside Service B like: "class ServiceB : IServiceB, ServiceA"?

    B.R,

    /M


    • Edited by moh85 Tuesday, April 7, 2020 10:29 AM
    • Moved by jakaruna-MSFTMicrosoft employee Tuesday, April 7, 2020 4:05 PM
    • Moved by Naomi N Tuesday, April 7, 2020 4:12 PM Design type of question - I think it may have a good answer in C#
    Tuesday, April 7, 2020 10:28 AM

All replies

  • I'd try asking for help over here.

    https://social.msdn.microsoft.com/Forums/windowsdesktop/en-US/home?forum=windowsgeneraldevelopmentissues

     

     



    Regards, Dave Patrick ....
    Microsoft Certified Professional
    Microsoft MVP [Windows Server] Datacenter Management

    Disclaimer: This posting is provided "AS IS" with no warranties or guarantees, and confers no rights.

    Tuesday, April 7, 2020 4:08 PM
  • Are you trying to use the repository pattern as a data persistence pattern, which is not its purpose? Are you trying to use the repository pattern as a domain pattern that is its purpose that has no data persistence logic?

    The repository should be used on a per business need, the domain,  and not a MVC controller need one repository per controller, like a repositroy object for Inventory, Sales, Purchase Order etc. and etc., which are domain needs.

    https://martinfowler.com/eaaCatalog/repository.html

    Are you trying to use the generic repository over EF, which is not optimal using the generic repository over the repository EF Core and non Core are already using?

    https://programmingwithmosh.com/net/common-mistakes-with-the-repository-pattern/

    https://dotnetcultist.com/repository-pattern-dead-with-entity-framework/

    https://blog.sapiensworks.com/post/2012/03/05/The-Generic-Repository-Is-An-Anti-Pattern.aspx

    I have many options like calling Service A inside Service B by injecting IService A in Service B constructor! or I can use inheritance like Service B can extend Service A then I can use all Service A methods inside Service B like: "class ServiceB : IServiceB, ServiceA"?

    What service are you talking about? The inheritance thing seems kind of questionable, and I don't see the need. However, there is nothing wrong in injecting an object into another object, becuase it promotes loose coupling, clean coding, better maintainability and unit testing. 

    Tuesday, April 7, 2020 7:21 PM
  • Thanks for reply ..

    Are you trying to use the repository pattern as a domain pattern that is its purpose that has no data persistence logic?

    It isn't for the presentation, I am using services to implement the business logic with many equations and calculations, so I divided these equations into multiple services and I am calling these services from the controller. Each equation depends on the previous one results, for that reason, I asked the question is it a good practice to use service inside another one?

    Wednesday, April 8, 2020 9:12 AM
  • Hi, this is a fun but not so easy question to answer. It really depends on what you are building and how much you need to reuse between components.

    Most important rule to me is to keep it as simple as possible, only introduce complexity if you need it. So if the application is small, doesn’t live long, is for a demo or something like that I don’t mind putting a lot of stuff in the controller and combine what I need there. But I do not let controllers call controllers, if something is needed from one controller in another that functionality is extracted in it’s own class and used from both controllers. But this can happen on every layer and so your architecture can grow. If you know from the start, that the project is going to be large and complex you can already start with introducing more layers.

    And to me that is key, thinking in layers. Any layer can call objects in the layer below it but doesn’t call objects in the same layer (some utility layers excluded) and never calls object in the layer above it.

    Then for your question you feel the need to let services call other services. To me that is a no go, because object should not call objects in the same layer. So I start looking for different solutions. And that begins with why I want to let on service call another service. In general, I see two reasons, the first is that I need to combine data, the second is that I need to combine processes (if this thing happened the other thing needs to happen also)

    If I need to combine data, I first look if I can combine it in another way, sometimes it is needed that we can combine the data already in the repository (it is almost always needed and fit’s good together). Sometimes it is just needed to show combined data on screen, in that case this becomes a viewmodel that I create within a controller based on several service calls.

    If the data doesn’t fit as a viewmodel or doesn’t fit in the repository I can introduce a composite Service (there are different kinds of composite services, with different names, look at the link below for more details). You could look at a composite service as a service that is allowed to call services and combine them, but composite services don’t call each other. The composite service also is the answer when the services calling each other is needed because of process. As a step in between I might consider the controller to have the function of composite service but as soon as the functionality is needed in more controllers it becomes a real composite service.

    Doing it this way can make it nice and clean and understandable. But sometime it can also result in almost empty controllers and empty services that do nothing else than passing the call through to the repository. If that happens a lot you might want to reconsider the choices made.

    The biggest problem people have with this is the lifetime of the database context and making it transactional sane. Most people start with either creating a db context in the controller and using it for that entire lifetime or in the repository. And for some applications that is just fine. But as soon as you need more control this can be messy. You might need a more complex solution where you introduce a separate “Unit of work” that is responsible for the lifetime of the db context and get created in your controllers or composite services because you want to make all combined calls transactional. DB context in itself is also a kind of “Unit of work” so sometimes it is enough to just carefully construct that in your controller or composite service but other times that is not enough.

    All this is managed by a dependency injection framework and object get the dependencies they need from their lower level injected. So a controller gets composite services or services injected, the composite service gets services injected, the services get repositories injected.

    https://www.martinfowler.com/eaaCatalog/unitOfWork.html

    https://emacsway.github.io/en/service-layer/


    Hope this helps,

    Here to learn and share. Please tell if an answer was helpful or not at all. This adds value to the answers and enables me to learn more.

    About me

    Wednesday, April 8, 2020 9:24 AM
  • Thanks for reply ..

    Are you trying to use the repository pattern as a domain pattern that is its purpose that has no data persistence logic?

    It isn't for the presentation, I am using services to implement the business logic with many equations and calculations, so I divided these equations into multiple services and I am calling these services from the controller. Each equation depends on the previous one results, for that reason, I asked the question is it a good practice to use service inside another one?

    So you are saying that you are using the MVC UI design pattern,  since you have mentioned the controller. Right?

    I don't see how the service pattern comes into play here or the repository pattern.  Can you post code on how you are using them?

    Wednesday, April 8, 2020 4:06 PM
  • Then for your question you feel the need to let services call other services. To me that is a no go, because object should not call objects in the same layer.

    The statement  defeats the purpose of using OOP, object interaction and using design patterns. A Purchase Order repository object  in the business layer can call upon the behavior of an Inventory repository object to complete a transaction.

    A Data Access Object DAOAuthor in the data access layer can call upon the behavior of the DAOArticle in the same layer. 


    • Edited by DA924x Wednesday, April 8, 2020 7:17 PM
    Wednesday, April 8, 2020 7:11 PM
  • Thank you all, it is really useful discussion, I will post snippet example of my code as the following:

    	

    // Class1

    public class PlanCalculation : IPlanCalculation { // DI private readonly PlanContext context; public PlanCalculation(PlanContext context) { this.context = context; } public PlanCalculationDto GetPlanCalculationReport(int userId) { var planCalculationFormula = context.PlanCalculations.First(u => u.UserId == userId); return new PlanCalculationDto { WorkingDaysPerYear = GetWorkingDaysPerYear(planCalculationFormula), WorkingHoursPerDay = GetWorkingHoursPerDay(planCalculationFormula), WorkingHoursPerWeek = GetWorkingHoursPerWeek(planCalculationFormula), ... WholeProductionPerHour = GetWholeProductionPerHour(planCalculationFormula), }; } public int GetWorkingDaysPerYear(PlanCalculation planCalculation) { return planCalculation.WorkingDaysPerWeek * 52; } public int GetWorkingHoursPerDay(PlanCalculation planCalculation) { return planCalculation.ShiftPerDay * planCalculation.WorkingHours; } ... public decimal GetWholeProductionPerHour(PlanCalculation planCalculation) { return (decimal)planCalculation.WholePercentage * planCalculation.ProductionPerHour; } }

     // Class2 public class ProductionFormulaCalculation : PlanCalculation, IProductionFormulaCalculation { private readonly PlanContext context; public ProductionFormulaCalculation(PlanContext context): base(context) { this.context = context; } public ProductionFormulaReportDto GetProductionReport(int userId) { var planCalculation = context.PlanCalculations.First(m => m.UserId == userId); var productionFormula = context.ProductionFormulas.First(m => m.UserId == userId); return new ProductionFormulaReportDto { ProductionPerHour = GetWholeProductionPerHour(targetPlantFormula) * productionFormula.WholePercentage1, ProductionPerDay = GetWholeProductionPerHour(targetPlantFormula) * productionFormula.WholePercentage2 }; } }

    I have more classes which have many equations and calculations that depend on the previous classes, so it is a chain of equations.

    Thanks in advance,


    • Edited by moh85 Thursday, April 16, 2020 5:28 PM
    Thursday, April 16, 2020 5:27 PM
  • So why is not called PlanCalculationRepo if this is a repositroy object?

    Why does it have a Dbcontext aka context being DI into it? If it's a true repository object, it should be calling  a mapping layer object like a DAO injected into it.

    https://martinfowler.com/eaaCatalog/repository.html

    https://martinfowler.com/eaaCatalog/dataMapper.html

    https://blog.sapiensworks.com/post/2012/11/01/Repository-vs-DAO.aspx

    https://javarevisited.blogspot.com/2013/01/data-access-object-dao-design-pattern-java-tutorial-example.html

    https://www.tutorialspoint.com/design_pattern/data_access_object_pattern.htm

    Here is DAO pattern using DTO pattern and Entity Framework. 

    using System.Collections.Generic;
    using System.Threading.Tasks;
    using Entities;
    
    namespace DAL
    {
        public interface IDaoAuthor
        {
            Task<List<DtoAuthor>> GetAll();
            Task<DtoAuthor> Find(int id);
            Task<List<DtoAuthorType>> GetAuthorTypes();
            Task Add(DtoAuthor dto);
            Task Update(DtoAuthor dto);
            Task  Delete(int id);
        }
    }
    ======================================================
    
    using System;
    using System.Collections.Generic;
    using System.Data.SqlClient;
    using System.Linq;
    using System.Threading.Tasks;
    using DAL.Models;
    using Entities;
    using Microsoft.EntityFrameworkCore;
    
    namespace DAL
    {
        public class DaoAuthor : IDaoAuthor
        {
            private PublishingCompanyContext pc;
    
            public DaoAuthor(PublishingCompanyContext dbcontext)
            {
                pc = dbcontext;
            }
    
            public async Task<List<DtoAuthor>> GetAll()
            {
                var dtos = new List<DtoAuthor>();
    
                var authors =  await pc.Author.ToListAsync();
    
                dtos.AddRange(authors.Select(author => new DtoAuthor()
                {
                    AuthorId = author.AuthorId,
                    FirstName = author.FirstName,
                    LastName = author.LastName
                }).ToList());
    
                return dtos;
            }
    
            public async Task<DtoAuthor> Find(int id)
            {
                var dto = new DtoAuthor();
    
                var author =  await pc.Author.FindAsync(id);
    
                if (author != null)
                {
                    dto.AuthorId = author.AuthorId;
                    dto.FirstName = author.FirstName;
                    dto.LastName = author.LastName;
                }
                else
                {
                    throw new Exception($"Author with ID = {id} was not found.");
                }
    
                return dto;
    
            }
            public async Task <List<DtoAuthorType>> GetAuthorTypes()
            {
                var dtos = new List<DtoAuthorType>();
    
                var authors = await pc.Author.ToListAsync();
    
                foreach (var author in authors)
                {
                    DtoAuthorType dto = new DtoAuthorType
                    {
                        AuthorId = author.AuthorId,
                        Value = author.AuthorId.ToString(),
                        Text = author.LastName + ", " + author.FirstName
                    };
    
                    dtos.Add(dto);
                }
    
                return dtos;
            }
            public async Task Add(DtoAuthor dto)
            {
                var author = new Author
                {
                    FirstName = dto.FirstName,
                    LastName = dto.LastName
                };
    
                pc.Author.Add(author);
                await pc.SaveChangesAsync();
    
            }
            public async Task Update(DtoAuthor dto)
            {
                var author = new Author
                {
                    AuthorId = dto.AuthorId,
                    FirstName = dto.FirstName,
                    LastName = dto.LastName
                };
    
                pc.Entry(author).State = EntityState.Modified;
                await pc.SaveChangesAsync();
    
            }
    
            public async Task Delete(int id)
            {
                
                var author = pc.Author.Find(id);
    
                if (author != null)
                {
                    var articles = await pc.Article.Where(a => a.AuthorId.ToString().Contains(id.ToString())).ToListAsync();
              
                    foreach (var article in articles)
                    {
                        author.Articles.Remove(article);
                    }
    
                    var payrolls = await pc.Payroll.Where(a => a.AuthorId.ToString().Contains(id.ToString())).ToListAsync();
    
                    foreach (var payroll in payrolls)
                    {
                        author.Payrolls.Remove(payroll);
                    }
    
                    pc.Author.Remove(author);
                    await pc.SaveChangesAsync();
                }
    
            }
    
        }
    }
    

    Thursday, April 16, 2020 6:39 PM