0

I read this article: http://peterspattern.com/dependency-injection-and-class-inheritance/ (great article btw)

Now, I have an ASP.NET Core web application, and I am thinking about putting all my services inside another class and use the built in DI to resolve this. Only reason I want to do this is to not have to manually update every constructor each time there is a need to add a new service to some class.

For example, instead of:

public class Foo {
IServiceOne _serviceOne;
IServiceTwo _serviceTwo;

 public Foo(IServiceOne serviceOne, IServiceTwo serviceTwo) {
   _serviceOne = serviceOne;
   _serviceTwo = serviceTwo;
 }
}

I can do this:

public interface IMyServices {
  IServiceOne ServiceOne {get;}
  IServiceTwo ServiceTwo {get;}
 }

public class Foo {
   IServiceOne _serviceOne;
   IServiceTwo _serviceTwo;

    public Foo(IMyService services) {
      _serviceOne = services.ServiceOne;
      _serviceTwo = services.ServiceTwo; 
    }
}

Edit: (I have left some parts out, like the class that implements IMyService and the registration of the service in Startup.cs)

Now, the application I am working on is quite large and the class that would be containing all the services could possibly contain 20-30 services.

My question is: is this a good idea? My concerns are with performance. Passing in all services in all constructors even if they are not needed feels like a bad idea. But it would save us tons of time if there would be only one class to update with a new service, then that service is available in all classes.

Is there perhaps someway to lazy load services when they are actually requested, or is that done by default?

Steven
  • 166,672
  • 24
  • 332
  • 435
Ashkan Hovold
  • 898
  • 2
  • 13
  • 28
  • 2
    If your classes contain too many dependencies, that's a good sign the class isn't following the single responsibility principle and is trying to do too much. – mason Feb 01 '18 at 15:06
  • Sounds to me as if you might want to first figure out whether your class actually needs 20-30 services or if you could split that up in the first place. Some other things to consider would be whether the injections should be transient, per request or singletons -- that will probably have an effect as well. I've worked on a website which had some parts where it used a single aggregation dependency object and that turned out to be a cause of slowness due to the excessive unnecessary resolving. Though you should probably measure those things to see if it's an actual issue in your case. – Jeroen Vannevel Feb 01 '18 at 15:06
  • 1
    Suggest you read up on SRP (Single Responsibility Principle) and SoC (Separation of Concerns). This has a code smell. – Nkosi Feb 01 '18 at 15:16

1 Answers1

2

Wow, that article you linked to combined a real-world problem with a solution to a different problem (constructor over-injection). It didn't address the original problem, which was using inheritance for a Controller in the first place.

It is really smelly to have to inject the same service into all of your controllers. This puts the problem squarely in the context of crosscutting concerns.

As I pointed out earlier in this answer, there are several ways to tackle this issue, the best of which in .NET Core are filters and middleware.

I suppose inheritance is also an option, but it is clearly the worst one, as it will inevitably lead to creating a god object.

NOTE: I am not saying that inheritance is not valuable in certain situations, but Controllers are generally not one of those situations.

NightOwl888
  • 55,572
  • 24
  • 139
  • 212
  • 2
    Funny thing is that [the author](http://peterspattern.com/dependency-injection-and-class-inheritance/) completely misinterpretted the [Aggregate Service pattern](http://blog.ploeh.dk/2010/02/02/RefactoringtoAggregateServices/). His solution is _not_ an implementation of an Aggregate Service. – Steven Feb 06 '18 at 12:14