51

I've recently been spending time reading up on SOLID principles and decided to seeing how the code base I work with compares.

In some of our code there is a repository (repository A). When a record is to be deleted from the repository A, we also need to delete an associated record from repository B. The original coder has therefore created a dependency to a concrete implementation of repository B. The method in repository A is within a transaction and deletes the record from repository A and then calls the method on repository B to delete the associated data.

My understanding of the S principle is that each object should have only 1 reason to change, but to my repository A has 2 reasons to change? Or am I way off the mark?

MrGrumpy
  • 543
  • 1
  • 4
  • 9

2 Answers2

91

Repository should have single responsibility - persist one kind of entity. E.g. employees. If you have to delete some associated records from other repository, it looks like business logic. E.g.

When employee is fired we should remove his work log

And usual place for business logic is a domain services. This service will have both repositories and do all the job:

staffService.Fire(employee)

Implementation will look like

public class StaffService
{
    private IEmployeeRepository employeeRepository;
    private IWorkLogRepository workLogRepository;
    private IUnitOfWorkFactory uowFactory;

    // inject dependencies

    public void Fire(Employee employee)
    {
        using(var uow = uowFactory.SartNew())
        {
            workLogRepository.DeleteByEmployee(employee.Id);
            employeeRepository.Delete(employee.Id);
            uow.Commit();
        }
    }
}

So, basic advises

  • try to keep your business logic in one place, do not spread part of it to UI, part of it to repositories, part to database (sometimes due to performance issues you have to do some logic on database side, but thats an exception)
  • never let repositories reference other repositories, repository is a very low-level component of your application with very simple responsibilities

You may wonder what to do if you have employee and it has some nested object which is stored in different database table. If you use that object separately from employee, then everything is as above - you should have separate repository, and some other object (service) which manipulates both repositories. But if you don't use that nested object separately from employee, then employee is an Aggregate Root and you should have only employee repository, which will query both tables inside.

Sergey Berezovskiy
  • 232,247
  • 41
  • 429
  • 459
  • 1
    Awesome answer @Sergey this confirms my thoughts, I spoke to the system architect and he suggested that it's a fine line between whether it's business layer or data layer but as it's cleaning up data it's fine to have it in the data layer. Hopefully I can convince him to move it up to the business layer – MrGrumpy May 08 '15 at 13:08
  • Deletion rules should usually be allowed to be eventually consistent rather than strongly consistent. Also, it is rather rare that a physical delete actually occurs. `employee.fire() -> EmployeeFired(employeedId) -> EventSubscriber -> ...`. – plalx May 08 '15 at 21:45
  • 19
    Repository should not reference another repository ? maybe, but suppose that I have ClassA that always need a nested ClassB (required to work), but the ClassB object itself can work alone. When my service retrieve a ClassA object from repositoryA, this repository would need to use repositoryB to retreive objB and assign it to objA (using a factory) like this: `class RepositoryA { public ClassA GetById(string id) { objB = repositoryB.GetById(idB); factory.CreateObjA(idA, objB); } }` How should it be done to avoid reference between the repositories ?? – Jonathan Dec 17 '15 at 19:53
  • My case is exactly like @Jonathan describes. What do you suggest in this scenario? – Rod Sep 03 '19 at 13:22
  • 2
    @Jonathan I don't think there is anything wrong with using repositories in other repositories in order to construct domain entities correctly. For example, If you have a meeting entity that cannot be constructed without a meeting type entity, then it's perfectly fine for the Meeting Repository to use the MeetingType repository. In fact, this still keeps isolation and you avoid code duplication. – L-Four Apr 24 '20 at 09:34
  • You should use the database context inside the repository to access the ClassB or use a ORM to map your database tables to your application entities where you can map the table relationship and access it's children via property accessors handled by the ORM mapping tool. – Machado Nov 30 '20 at 17:54
-4

In this case you should make use of the event dispatcher pattern.

After the delete operation on RepoA you can dispatch an event like:

dispatch repositoryA.deleted(RecordA)

that will hold the information of the deleted record.

An vent listern then will subscribe on such event and, having Repository B as dependence will then invoke a delete.

Let's use B as the entity name, the listener declaration should sound like:

Listen RepositoryA.delete and invoke onDelete(Event)

With this approach you have realized a loose coupling between repoA and repoB (enforcing the Open/Close principle - on the close side-) so repoA have now (again)

regards.

Francesco Panina
  • 314
  • 4
  • 16
  • I downvoted because this is a clear example of overengineering. This solution pulls in events where they are not needed and the question can be solved with simpler means. – Hop hop May 03 '23 at 13:06