Best Practice to Get 1 object from Database using WebApi C# Layered Architecture

Question

So I know my question seems basic but I want to know something that's been bugging me for a while, My backend is done following the Layered Architecture(repo-services-controllers)

I have an api call that should return a json of an employee after providing his id, so the url is something like api.mywebsite.com/api/employees/1

and my controller will look like this:

public async Task<EmployeeDto> GetEmployee([FromUri] int eId)
{
    return GetService<IEmployeeService>().GetEmployeeById(eId);
}

my question is, what are the checks I'm supposed to do when I get this object? Should I do a check if the employee is deleted (soft deleted that is)? I obviously should do a check if it returns a null (didnt find an employee with such an id)

But if I want to do a check if the entity is deleted, should I do it in the repository layer or the service layer? repo layer:

public Task<Employee> GetSingle(int id)
{
    return GetDatabase().Employees.Where(x => x.EmployeeId== id && !x.Deleted).SingleOrDefaultAsync();
}

or on the service layer:

var emp= await GetTenantRepository<IEmployeeRepository>().GetSingle(eId);
if (emp==null)
{
    throw ...
}
if (emp.Deleted)
{
    throw ...
}

Am I too overthinking it and it doesnt matter if I put it here or there?

As an FYI: `GetService()` is known as the service locator **anti-pattern**, learn about Dependency Injection if you care about best practices. The condition checked on the database makes more sense since otherwise you would be getting an entity that would be wasted (since you wouldn't return it to the client) — Camilo Terevinto, Aug 30 '18 at 11:12
I know how the GetService and DI work (at least I think), and yeah I think I should do the check on the repo layer, that way like you said, if its deleted I won't get something that would go to waste — Ali_Nass, Aug 30 '18 at 11:14
@CamiloTerevinto I might have miss understood you, in my controller I used the unit of work to get me the services I need like I mentioned above, something like this, public async Task GetEmployeeById([FromUri]int employeeId) { return await unitOfWork.GetService().GetEmployeeById(employeeId); } are you saying that using the service locator (the get service using the unit of work) is a bad practice? — Ali_Nass, Aug 30 '18 at 11:17
Yes, it is a bad practice and it's discouraged. Read [here](https://stackoverflow.com/questions/4985455/dependency-injection-vs-service-location) — Camilo Terevinto, Aug 30 '18 at 11:22
There is no answer to this question. *Am I too overthinking it and it doesnt matter if I put it here or there?* Yes, put it where it seems like the best place to put it and move on with your life — Liam, Aug 30 '18 at 11:50
I'd also advise that you read up on other [design patterns](https://www.dofactory.com/net/design-patterns). "3 layer" is an anti pattern IMO. It's touted as the panacea to all design, but there is no one pattern you should use everywhere. — Liam, Aug 30 '18 at 11:58
You should look at this from the top-level scenarios stand point. In other words the answer to the first 2 questions lays in the area of questions "how application should behave when an employee requested", "what should happen with application if a requested employee is not found", etc. The last question is subject of responsibility assignment. Considering that the employee existence verification is rather business scenario I'd say it's responsibility of a business layer (in your example probably service). — Dmytro Mukalov, Aug 30 '18 at 12:11

n.prokhorov · Answer 1 · 2018-08-30T11:53:27.077

It's all about code style, architecture philosophy and chosen design :) Answering on your question about "where to put checking": It depends on responsibilities of your layers. All code samples are just for reference and shouldn't be considered as straightforward solution

1) DAL (repository) - is the bridge from application to storage. It should encapsulate all the stuff related to DB interaction and DB implementation details. So, in case of missing entity it's fine to return null from repository interface.

2) BL (service) - is combining raw data fetched from repository(s) and applying some business rules/actions to them. In case of simple CRUD service it becomes very similar to repository and may handle some DB exceptions, make data transformations (map DAL object to BL) etc. Again, null returned from BL layer indicates missing entity. E.g. based on your code you can encapsulate some DB inconsistency here and consider multiple entries as missing value. BL service example:

public async Task<EmployeeDto> GetEmployee(int eId)
{
    try
    {
        return GetService<IEmployeeService>().GetEmployeeById(eId);
    }
    catch (InvalidOperationException) //If it's suitable
    {
        return null;
    }
}

3) API layer (controllers) - is the RESTFUL interface. It combines results from service(s) and represents it as resources and returns status codes and resources represenation. In case of missing entity the best way is to return 404 status (HttStatusResult) from your RESTFUL API.

public async Task<IHttpActionResult> GetEmployee([FromUri] int eId)
{
    var res = GetService<IEmployeeService>().GetEmployeeById(eId);

    return res == null ? (IHttpActionResult)NotFound() : Ok(res);
}

So, you are free to check the missing result on ANY layer, but do it in the right way. Your repository should never drive the API behavior(I mean no code should be developed for resources on DAL. E.g. resource object should not have exactly the same props as DAL object) , BL never should affect DAL and vice versa.

P.S. it goes beyond the scope of question, but take a look at IoC and DI. You current DI implementation is not the best in my opinion.

"You current DI implementation is not the best in my opinion.", there's no DI in the question's code — Camilo Terevinto, Aug 30 '18 at 11:55
Original question is about 3-layerd architecture, proposing ideas "total rework and problem should be gone" are real irrelevant. My post is an answer on very questions asked. — n.prokhorov, Aug 30 '18 at 11:58
Ok, I'll accept that, but still 1,2 and 3 are adding nothing. The only relevant part here is *you are free to check the missing result on ANY layer*, which doesn't seem like a particularly helpful answer — Liam, Aug 30 '18 at 11:59

Best Practice to Get 1 object from Database using WebApi C# Layered Architecture

1 Answers1