CODEWITHSUNDEEP
asp.net-mvcdependency-injectioncastle-windsor
JTW
JTW

Reputation: 3685

DI with constructor injection: Am I injecting too many services?

Using Castle Windsor for DI, in my MVC controller, I have the following: 

    public IAccountService AccountService { get; set; }
    public IPasswordService PasswordService { get; set; }
    public IEmailService EmailService { get; set; }

    public AccountController(IAccountService accountService,
        IPasswordService passwordService, IEmailService emailService)
    {
        AccountService = accountService;
        PasswordService = passwordService;
        EmailService = emailService;
    }

The Account service is doing standard auth stuff, the password service is used to reset the users password, and the email service is used to send confirmation emails to the user. My question is this: am I injecting too many services in this constructor? Is there a significant negative impact of doing this? If so, what is a better approach? Thx.

Upvotes: 3

Views: 1364

Answers (2)

X3074861X
X3074861X

Reputation: 3819

What you have looks completely fine, aside from Phil's comment about making those setters private. In fact, there are many instances, especially with an account controller, where you're going to need a lot more than just 3 services injected.

Where you'd want to exercise caution would be with what you're injecting, and weather or not it belongs directly in the controller in the first place.

Other than that, as long as you're using the design pattern appropriately (which you are), you don't have anything to worry about.

Upvotes: 1

Phil Sandler
Phil Sandler

Reputation: 28016

This is really a matter of opinion, but I don't think many people would have a problem with three injected services.

The rule of thumb that I learned early in my career was that more than five parameters to any method (including constructor methods) is too many. I've since seen arguments that more than three is too many.

Injecting too many services could be a symptom that your class has too many responsibilities, but it might just be a symptom of what those responsibilities are. I would personally never refactor a class for the sole purpose of injecting fewer dependencies, but would definitely see this as a reason to review the code and make sure it wasn't a symptom of some other problem.

As an aside, you should make your setters private (or even better: use a read-only member variable) for the services that are injected--generally you don't want to expose them to change from the outside.

Upvotes: 7

Related Questions