Putting some business logic in repositories versus keeping it out of repositories entirely?

Question

I know most (if not all) business logic should reside in its own layer, but what is the general consensus of putting some basic business logic inside of the repository layer itself?

My scenario: We have a table that has a few optional columns, but should one of the columns contain a value, the other two will be influenced by that value.

A POCO of an entity for that table looks something like:

public class Template
{
     public int Id {get;set;}
     public string MessageTemplate {get;set;}
     public int? FacilityId {get;set;}
     public int? ResourceId {get;set;}
     public bit GlobalDefault {get;set;}
}

Now there should only be one entry that is designated as the global default, and similarly, each facility or resource should only be listed in the table once. The latter constraints are enforced by SQL Server, but this does require some business logic to make sure everything flows smoothly.

In this table's case, should we attempt to insert a new record for an existing facility or resource, the previous record should either be updated instead (or deleted). The same is true for setting a new global default. Only one entity should be the global default so if another exists it should either be deleted or updated instead.

Now obviously, my business layer is going to handle these scenarios to the best of their ability, but how frowned upon is it to also implement that level of logic in my repositories themselves?

I'm envisioning something like the following in my repo:

public void InsertFacilityTemplate(Template template) 
{
    var existing = context.Templates.Where(n => n.FacilityId == template.FacilityId);
    Template recordToSave;
    if (existing.Count > 1) 
        DeleteRecords(existing);
    if (existing.Count == 1)
        recordToSave= existing.First();

    if (recordToSave != null)
    {
        recordToSave.MessageTemplate = template.MessageTemplate;
        Update(recordToSave);
    }
    else
        Insert(template);
}

What is the proper approach here?

Answer 1

Databases and methods often use the term upsert for actions that either insert OR update based on existence. It's fine to have an upsert method in a repository.

It's not clear from you description if you are asking about repeating the insert/update logic elsewhere, or you are asking where to put this logic (i.e. example method).

• Repetition of the logic at a previous or subsequent step is generally a poor practice. If this logic is repeated elsewhere, then there should be a different effect such as giving the end user an error/warning message and preventing further action. Handling elsewhere for the purpose of "more checks = better chance of catching" is an indication that the logic is held together by hope instead of intentional handling.
• Your example could use more "separation of concerns" which relates to where some of the logic goes. Although the repo method has the name "InsertX", it also updates and deletes. The delete here is a side effect which is a frowned upon practice when unintended or unclear. Consider calling a separate method that makes the upsert preparation more clear such as RemoveDuplicateInstances(ITemplate template) or PrepareUpsert(string templateId). The example also assumes to know what the updated field is. For example, if the GlobalDefault value was toggled, or another field was added to the class then the Insert would preserve those changes while the logic to the Update wouldn't. That would be inconsistent behavior with the expectation.

Those concerns being said, if your application is small, proof-of-concept, or exceedingly simple then these change are going to be so minor that it won't functionally make a difference. It will make a difference if someone else is coming after you or the intention is to continue expanding this code base.

Answer 2

The root of the problem seems to be an Anemic Domain Model. There is nothing wrong with having an POCO class to represent the data model. The problem you are trying to solve is derived from the following business rules:

1. A "facility" can have multiple templates, as long as each template for that facility references a unique "resource".

2. Only 1 of the templates for a facility should be the "global default".

Given these rules you need to push this logic into a class that has a handle on a "facility" and all of its "templates."

Something like:

var facility = // get by facility Id
var template = new Template() { /* initialize properties */ };

facility.SetGlobalTemplate(template);

Then the SetGlobalTemplate method loops over the templates associated with that facility, and sets the bit flags accordingly.

Then your "repository" just needs to save the new facility. Make sure your data mappings can pick up the new template and the update to the former global template. Any halfway decent ORM should be able to do this (Entity Framework, NHibernate, etc).

repository.SaveFacility(facility);