Multiple doubts about Repository pattern

I have implemented the Repository pattern in my Web APi 2 + Entity Framework 6.1 + Angular 1.4 SPA site. The application allows management of a martial arts club. One of its functionalities is to allow to handle belt promotions. This involves some validations i.e. You can't add a promotion to a higher belt with a date lower than an existing promotion to a lower belt. Currently i have this validation as part of the repository. This is obviously wrong. I have a couple of questions:

• Where should I put this validation? Should it be in the controller?

• Should I create a separate class for validation and use it in controller?

• How should i return potential validation errors to the front end? Should it be an exception with a message?

• Is it a good practice to return results of more complex validations from the API and display those messages in the UI?

• Should I have one repository as controler field or instantiate it with each request? Does't instantiating with each request create a risk of issues with database connection?

Here is my current implementation of the repository and controller classes:

```
public class PromotionsRepository : RepositoryBase, IPromotionsRepository
{
public PromotionsRepository()
{
}

public async Task> GetPromotionsForMember(int memberId)
{
using (var context = new BjjClubEntities())
{
try
{
var promotions = await context.Members.Include("Promotions").Where(member => member.MemberId == memberId).SingleOrDefaultAsync();
return promotions.Promotions.OrderBy(promotion => promotion.Date).ToList();
}
catch (Exception ex)
{
LogError(ex);
return null;
}
}
}

public async Task AddPromotions(int memberId, List newPromotions)
{
using (var context = new BjjClubEntities())
{
try
{
newPromotions.ForEa

First, some overall considerations:

catch (Exception ex)
        {
            LogError(ex);
            return null;
        }

It is usually a bad practice to return null when a exception happens. The consumer of your method will think that there are no results (in this case, no promotions for member), when what actually happened is that the connection has time-outed and he need to retry...

await _promotionsRepository.AddPromotions(memberId, newPromotions);

As @Disappointed commented, if there is nothing in parallel, there is no reason to use async/await.

Now, to your questions:

• Where should I put this validation? Should it be in the controller?

It is usually a good idea to separate your business logic from everything else, so changes on it does't spread throught all your application. So, the validation should be on your business layer. The controller gets the data from the repository, send it to the appropriate class on the business layer, which execute the rules on that data, and return the results to the controller.

• Should I create a separate class for validation and use it in controller?

Yes, as per answer 1.

• How should i return potential validation errors to the front end? Should it be an exception with a message?

That depends on your rule. If your method on the business layer is called "AddPromotions", it should either add a promotion or throw a exception. If your method is called "CanBePromoted" it should return a bool indicating if your user can be promoted, and throw if it is unable to determine whether the user can be promoted or not. In the end, it is all about the contract you are setting.

But when you say "validations", I'm pretty sure that you mean the "AddPromotions" situation. On this case, it is usually better to just throw. If you need to validade multiple rules and return all the issues at once, take a look at the class AggregateException, it is a exception that contains a list of exceptions inside.

• Is it a good practice to return results of more complex validations from the API and display those messages in the UI?

Well, sure, your user must know what he did wrong, right? But be sure to show the user just meaningful business errors: No null references, no database issues, no nothing that means "computer stuff went wrong". You are building a martial arts management program, so only messages that a martial arts teacher would understand should be displayed. A good way to do it is to have a specific exception type that you use to throw your validations, and make a global catch that return the text of this kind of exceptions to the UI. For all the other exceptions that this filter catch, return a generic error message and log the actual exception.

• Should I have one repository as controller field or instantiate it with each request? Does't instantiating with each request create a risk of issues with database connection?

On each request, you are correct on your assumption that it will create issues with the DB connection. Probably a lot of them.

StackExchange Code Review Q#152693, answer score: 3