ASP.NET Core DI Constructor vs RequestServices [duplicate]

Question

Why is requesting services via HttpContext.RequestServices or IServiceProvider consider bad practise. I can read this statement all over the place:

It is recommended to use constructor injection instead of getting it using RequestServices.

---

My idea is just the opposite. Use RequestServices where ever possible. Let me explain why. I want to keep controllers as small as possible so I create separate services. This way, my controllers are clean:

public class MyController : Controller
{
    public MyController(IMyServices MyServices){}

    public async Task<IActionResult> GetSomething(int parameter)
    {
       return await this.MyServices.SomeMethod(parameter);
    }
}

All services inherit from base, which contains logic for managing rights, caching sql requests ... .
With constructor approach I get very complicated system of calling base:

public class MyBaseService
{
    public MyBaseService(ILogger Logger, AppDbContext Context, IMemoryCache Cache) { }

    public bool HasRight(string right) { return true; }

    public bool IsLoggedIn() { return true; }
}

public class MyServices : MyBaseService
{
    public MyServices(ILogger Logger, AppDbContext Context, IMemoryCache Cache) : base(Logger, Context, Cache)
    {    
    }
}

but with GetRequiredService I simplify calling base on constructor:

public class MyBaseService2
{
    private ServiceProvider _ServiceProvider;
    public MyBaseService2(IServiceProvider ServiceProvider)
    {

    }

    protected ILogger Logger { get { return this._ServiceProvider.GetRequiredService<ILogger>(); } }

    protected AppDbContext Context { get { return this._ServiceProvider.GetRequiredService<AppDbContext>(); } }

    protected IMemoryCache Cache { get { return this._ServiceProvider.GetRequiredService<IMemoryCache>(); } }

    public bool HasRight(string right) { return true; }

    public bool IsLoggedIn() { return true; }
}

public class MyServices2 : MyBaseService2
{
    public MyServices2(IServiceProvider ServiceProvider) : base(ServiceProvider)
    {    
    }
}

Yes, BaseService contains more code, but when I need additional services in BaseService, there is no need to fix every class base constructor call. Also all my services have much simpler constructor (just IServiceProvider).

If I go against constructor approach. Is there any performance hit if calling ServiceProvider.GetRequiredService for MyServices if there lifetime is Scoped.

Answer 1

Why is requesting services via HttpContext.RequestServices or IServiceProvider consider bad practise.

It's considered a bad practice because it is an implementation of the Service Locator anti-pattern.

You are trying to keep your controller classes small by moving many common dependencies and common logic into the base class, but that is an anti-pattern by itself, because:

• Even though dependencies are resolved from the base class, those dependencies are still hidden, as this article explains. This means dependencies are hidden from your unit tests and from the DI Container, who will be unable to do analysis on your object graphs.
• Moving dependencies to the base class doesn't lower the complexity of a class, since the base class is always tightly coupled with the derivative.
• It causes many responsibilities to be crammed into the base class, which will cause the base class to violate the Single Responsibility Principle. This can force the base class to become a god class and under constant change.

Base classes use inheritance, while the general consensus in software development is that you should favor Composition over inheritance.

Since you should favor composition, this automatically leads to Dependency Injection. Without having a base class, it immediately becomes easier to spot Single Responsibility Principle violations, because your constructors will get more arguments. Note again that the number of dependencies doesn't change when using Service Locator, they are just harder to count.

You should embrace the fact that Constructor Injection easily leads to Constructor over-injection, since this is an indication that our class does too much and it is a signal that we should redesign such class.

Instead of implementing cross-cutting concerns such as logging, caching, and authorization in a base class, implement them using composition. For instance, you can use middleware, decorators or interceptors to apply cross-cutting concerns to a request (or a specific service of such request).