2

I have an application with dependency injection that consumes a REST API. The API calls are abstracted into entity-specific repositories. Some of the repositories are: HttpNotebookRepository, HttpPageRepository, HttpBookmarkRepository, HttpUserRepository and their corresponding interfaces.

I have a NotebookHierarchyController which requires four of these repositories via constructor injection:

// constructor
public NotebookHierarchyController(
    NotebookRepository notebookRepo, 
    PageRepository pageRepo, 
    BookmarkRepository bookmarkRepo, 
    UserRepository userRepo, <other dependencies>
) { .. }

There a multiple such controllers requiring more than one or two repositories. I have been planning to create a composite repository as follows:

public class CompositeRepository {
    private NotebookRepository notebooks;
    private PageRepository pages;
    private BookmarkRepository bookmarks;
    private UserRepository users;
    
    public CompositeRepository(NotebookRepository notebooks, 
        PageRepository pages, 
        BookmarkRepository bookmarks, 
        UserRepository users) {
        // assign to fields
    }

    public NotebookRepository notebooks() {
       return notebooks;
    }

    public PageRepository pages() {
       return pages;
    }

    public BookmarkRepository bookmarks() {
       return bookmarks;
    } 

    public UserRepository users() {
       return users;
    }
}

And then inject it into my controller:

// constructor
public NotebookHierarchyController(CompositeRepository repo) { .. }

I now easily have access to all repositories:

repo.notebooks().updateName(..);
repo.sections().add(..);
...

I have people do this in some implementations of the Unit of Work Pattern. However, as far as I understand, the goals of a unit of work are different. It is primarily for transaction coordination. This may also be different from wrapping the repository in a service to add business logic and decoupling.

What I've done is a simple grouping of repositories into one service for convenience. Is this an anti-pattern? Or should I prefer individually injecting each repository? Or is there a more accepted and standard way to deal with this?

Amal K
  • 91
  • 11
  • 2
    "The API calls are abstracted into entity-specific repositories" - that's fine, and what a lot of people do, but really, the repos should not be entity-specific (should not necessarily map 1-to-1 to entities). Rather, they should be specific to various *usage scenarios*, or perhaps specific to some finer grained business-tasks, and might involve a single entity multiple entities, or even only partial entities. *That's* how you get abstraction; then you mostly inject the one scenario-specific repository into the class that implements associated business-logic. – Filip Milovanović Mar 10 '23 at 15:41
  • 2
    Otherwise, you're doing little more than adding a fairly thin wrapper around the ORM - it doesn't really create a sufficient level of isolation, cause the interface ends up being too similar (and thus coupled) to that of the ORM itself (if not in detail, then conceptually - but the effect is the same). – Filip Milovanović Mar 10 '23 at 15:44
  • 2
    Hiding several dependencies behind a wrapper doesn't provide a significant improvement or advantage other than "it's faster to inject". This reminds me of the ServiceLocator a pattern many would consider to be anti-pattern. – Laiv Mar 11 '23 at 09:25
  • **(1)** What is the concrete problem you're trying to solve? Keeping the character count as low as possible? Because you haven't actually said why your current solution is somehow bad or to be avoided. **(2)** Having your controller directly access your DAL is in my opinion a problem, or at the very least questionable _especially_ if you need logic that starts using multiple repo's to generate a single answer; and solving this problem first would preclude the question (as the inbetween application layer would do the repo-bundling for you) – Flater Mar 16 '23 at 05:21

1 Answers1

1

You are doing fine. What you have created is a simple Facade service. It is a standard way of reducing a growing number of constructor parameters (see this older SE.SE answer here).

The only thing which is debatable is the class name. CompositeRepository sounds very generic for a facade specialized for a NotebookHierarchyController. I personally would prefer a more specialized name like NotebookHierarchyRepoService (but YMMV).

Doc Brown
  • 199,015
  • 33
  • 367
  • 565