24

Introduction

Hi everyone, I'm currently working on a persistence library in C#. In that library, I have implemented the repository pattern where I'm facing a SOLID issue. Here is a simplified example my current implementation to focus on the essential:

The abstract repository containing in the persistence library:

public abstract class Repository<T> 
{
    protected Repository(
        IServiceA serviceA,
        IServiceB serviceB) 
    {
        /* ... */
    }
}

The concrete repository created by the library user:

public class FooRepository : Repository<Foo> 
{
    protected FooRepository(
        IServiceA serviceA,
        IServiceB serviceB) :
        base(serviceA, serviceB)
    {
        /* ... */
    }
}

Problem

OK, with the current code, the derived class has to know every dependency of the base class which can be ok, but what if I add a dependency to the base class? Every derived class will break because they will need to pass that new dependency to the base class... So currently, I'm limited to never change the base class constructor and it's a problem because I want my base class to had the possibility to evolve. This implementation clearly breaks the Open/Closed Principle, but I don't know how to solve this issue without breaking the SOLID...

Requirements

  • The library should be easy to use for the user
  • The concrete repositories should be able to be constructed through the DI
  • One or more dependencies should be added to the abstract repository without impacting the derived repositories
  • It should be possible to register every repository in the DI container using a naming convention as does the ASP.NET MVC framework with the controllers
  • The user should be able to add more dependencies in his derived repository if he wants

Solutions already envisaged

1. Service aggregator pattern

Following this article, the service aggregator model can be applied in this situation so the code would look like something like this:

The abstract repository containing in the persistence library:

public abstract class Repository<T> 
{

    public interface IRepositoryDependencies
    {
        IServiceA { get; }
        IServiceB { get; }
    }

    protected Repository(IRepositoryDependencies dependencies) 
    {
        /* ... */
    }
}

The concrete repository created by the library user:

public class FooRepository : Repository<Foo> 
{
    protected Repository(IRepositoryDependencies dependencies) :
        base(dependencies)
    {
        /* ... */
    }
}

Pros

  • The derived classes don't break if a dependency is added to the base class

Cons

  • The implementation of the IRepositoryDependencies interface has to be modified if we add a dependency
  • The article doesn't explain how to use Castle DynamicProxy2 (which is an unknown technology for me) to dynamically generate the service aggregator

2. Builder pattern

Perhaps, it's possible to remove the base repository constructor and introduce a builder template to create the repositories, but for this solution to work, the builder must be inheritable to allow the user to enter his repository own dependencies.

Pros

  • The derived classes don't break if a dependency is added to the base class
  • The repositories construction is managed by another class

Cons

  • The user has to create a builder for each repository he wants to create
  • It's become harder to register every repository through the DI using a naming convention

3. Property injection

Perhaps removing the base repository constructor and configuring the DI to use property injection might be an option.

Pros

  • The derived classes don't break if a dependency is added to the base class

Cons

  • I think the property setter must be public?

Conclusion

Is there any of the mentioned solutions that could be acceptable in a SOLID world? If not, do you have a solution for me guys? You help is very appreciated!

Improve this question
12
  • 3
    What does the new dependency of the base class do? If the base class requires a new dependency that suggests that it's going to have additional responsibility. IOW - what is it going to do that it couldn't do before without an extra dependency? Could it be that the new behavior doesn't belong in that class at all? Commented Jan 1, 2018 at 0:23
  • @ScottHannen If I want to add a simple logger for example. Also, I currently have a dependency to register the repository through the unit of work, but what if in the future I don't need that anymore? Is it really ok too let the derived class know every dependency of the base class? Commented Jan 1, 2018 at 0:49
  • "This implementation clearly breaks the Open/Closed Principle" I'm not sure if I agree with you here. For me, if you need to make changes to your base class then derived class should be passing what it takes to instantiate the base class. If the derived class does not need the dependency and it is a perfectly valid case, then maybe you should create an additional constructor without taking the dependency. Or create another base class which can be derived from original base class with additional dependency for the derived classes which needs the that dependency Commented Jan 1, 2018 at 0:58
  • If your base class has multiple constructors, then I as the user would decide which one to use. If I need the logger, then I will call the constructor with the logger. You just have to make sure that the implementation works and the class is useful for each constructor version. In the future, if you need to add even additional responsibilities, well you should not because then you will break the SRP. Commented Jan 1, 2018 at 0:58
  • @AnkitVijay Ok, but what if the way of building the base class change? It's possible to change the way a class is built without adding or removing his responsibilities and if it occurs every derived class will have to change? If I add a dependency I can add a constructor I understand that and you're perfectly right, but what if a required dependency change? Should I never change a required dependency? Commented Jan 1, 2018 at 1:10

4 Answers 4

Reset to default
10

After some years of experience, I found the Decorator Pattern a perfect fit for this.

Implementation:

// Abstract type
public interface IRepository<T>
{
    Add(T obj);
}

// Concete type
public class UserRepository : IRepository<User>
{
    public UserRepository(/* Specific dependencies */) {}

    Add(User obj) { /* [...] */ }
}

// Decorator
public class LoggingRepository<T> : IRepository<T>
{
    private readonly IRepository<T> _inner;

    public LoggingRepository<T>(IRepository<T> inner) => _inner = inner;

    Add(T obj) 
    {
        Console.Log($"Adding {obj}...");
        _inner.Add(obj);
        Console.Log($"{obj} addded.");
    }
}

Usage:

// Done using the DI.
IRepository<User> repository = 
    // Add as many decorators as you want.
    new LoggingRepository<User>(
        new UserRepository(/* [...] */));

// And here is your add method wrapped with some logging :)
repository.Add(new User());

This pattern is awesome, because you can encapsulate behaviors in separate classes without breaking changes and using them only when you really need them.

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

This does not let you reuse method implementations in the concrete class.. as in you have to explicitly redefine all the methods in IRepository. If I had inherited from UserRepository, then I could just call Add directly (no need to reimplement).
@VisheshAgarwal If you want to avoid redefining all the methods in IRepository, add a base class named RepositoryDecorator with all virtual members. But keep in mind that if redefining all methods bother you it might be because your interface have too many methods/responsabilities.
6

As asked by you, here is one very basic and crude sample of solving this problem through Composition rather than inheritance.

public class RepositoryService : IRepositoryService
{

    public RepositoryService (IServiceA serviceA, IServiceB serviceB) 
    {
        /* ... */
    }

    public void SomeMethod()
    {
    }     
}

public abstract class Repository
{
    protected IRepositoryService repositoryService;

    public (IRepositoryService repositoryService)   
    {
      this.repositoryService= repositoryService;
    }

    public virtual void SomeMethod()
    {
          this.repositoryService.SomeMethod()

          .
          .
    }
}

public class ChildRepository1 : Repository
{

    public (IRepositoryService repositoryService)  : base (repositoryService)
    {
    }

    public override void SomeMethod()
    {
          .
          .
    }
}

public class ChildRepository2 : Repository
{

    public (IRepositoryService repositoryService, ISomeOtherService someotherService)   : base (repositoryService)
    {
          .
          .
    }

    public override void SomeMethod()
    {
          .
          .
    }
}

Now, the abstract base class and each child repository class here will only depend on IRepositoryService or any other required dependency (refer ISomeOtherService in ChildRepository2).

This way your child repository only supplies IRepositoryService dependency to your base class and you do not need to supply the dependencies of IRepositoryService anywhere.

Improve this answer

4 Comments

This implementation breaks one of my requirement ("The library should be easy to use for the user"). It's annoying for the user to have to do this for each method of the RepositoryService. I'm looking for something more like the DbContext class in EF Core (a parameterless constructor and a way to change the default options)
But, you will own RepositoryService and Repository class, not your user. So, I'm not sure how it is difficult for your users to use.
Oh I didn't read the code correctly.. It can be I way to do it! Thanks!
If this has helped you resolve your query, I would ask you to mark this as answer
2

It might be a bit late, but you can use the IServiceProvider in the constructor instead:

The abstract repository containing in the persistence library:

public abstract class Repository<T> 
{
    protected Repository(IServiceProvider serviceProvider) 
    {
        serviceA = serviceProvider.GetRequiredService<IServiceA>();
        serviceB = serviceProvider.GetRequiredService<IServiceB>();
        /* ... */
    }
}

The concrete repository created by the library user:

public class FooRepository : Repository<Foo> 
{
    protected FooRepository(IServiceProvider serviceProvider)
        :base(serviceProvider)
    {
        serviceC = serviceProvider.GetRequiredService<IServiceC>();
        serviceD = serviceProvider.GetRequiredService<IServiceD>();
        /* ... */
    }
}

This way each of the classes can utilize their own services without any cross-dependency.

There are downsides to this approach. First, this is a sort of service locator (anti)pattern with an abstract locator, though that does not mean it's not supposed to be used anywhere. Just be judicial about it, and make sure there isn't a better solution. Second, there is no compile-time enforcement to supply the necessary service implementations to any of the repository classes. Meaning, this will still compile if you do not put concrete implementation of IServiceA into the service provider. And then, it will fail run-time. However, in this case, that was one of your requirements.

Improve this answer

1 Comment

It's pretty much like the service locator pattern which is kind of an anti-pattern.
-3

The conclusion "I'm limited to never change the base class constructor " is pure demonstration how heavy IoC container "pattern" is toxic.

Imagine you have an asp application and want to have the possibility to enable logging for specific user the way when each his session goes to new file. It is impossible to achieve with IoC Containers / Service locators / ASP Controllers constructor. What you can do: on each session start you should create such logger and accurately pass it through all constructors (service realizations etc deeper and deeper). There are no other ways. There are no such things as "per session" life cycle in IoC containers - but this is only one way which instances naturally should live in ASP (means multiuser/multitask app).

If you do not have DI through constructor you are definitely doing something wrong (this is true for ASP.CORE and EF.CORE - it is impossible to watch how they torture everybody and themself with abstraction leaks: can you imagine that adding custom logger broke the DbContext https://github.com/aspnet/EntityFrameworkCore/issues/10420 and this is normal?

Get from DI only configuration or dynamic plugins (but if you do not have dynamic plugins do not think of any dependency "as it could be a dynamic plugin") and then do all DI standard classic way - through constructor.

Improve this answer

5 Comments

Sorry but I don't really understand what is your proposition here (maybe it's my bad English)? You tell me to not thinking about the possible evolutions that could occur in the base class? You're also talking about EF Core. EF Core do it fine right? I'm wondering how EF Core DbContexts are built when the base constructor isn't called? How the connection string or the EF provider are collected for example?
ef core shows the wrong way. What I say just pass all DI through the constructors. If you can't do it - you definitely doing something wrong.
Of course I can but this sample will break your imaginable rule "The library should be easy to use for the user" - where you decide what is easy what not (and it is not neutral). I try to answer with seeding different idea to your head: imagine that all your objects (logger and dependent on logger that means all) need to live "per session". What would you do?
I want the user to inherit from the base repository and maybe some easy configuration and that's it. The user should not write much code to create a repository. If I refer to EF, it's possible to change the logger factory and the internal service provider in the options. So it's possible to pass some "per session" services to the DbContext if we want? Why it's bad to do something like? I mean the options are fully configurable?
logging "per session" was normal with EF6 (you was able to create logger per session and subscribe it to the Messages event), and it is possible but extrimely complex in EF Core : stackoverflow.com/questions/43680174/… (read my answer). Compare one line event subscribtion with hundred lines of pooling loggers in ef core that uses super-puper IoC containers .

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.