Does dependency injection increase my risk of doing something foolish?

Question

I'm trying to embrace widespread dependency injection/IoC. As I read more and more about the benefits I can certainly appreciate them, however I am concerned that in some cases that embracing the dependency injection pattern might lead me to create flexibility at the expense of being able to limit risk by encapsulating controls on what the system is capable of doing and what mistakes I or another programmer on the project are capable of making. I suspect I'm missing something in the pattern that addresses my concerns and am hoping someone can point it out.

Here's a simplified example of what concerns me. Suppose I have a method NotifyAdmins on a Notification class and that I use this method to distribute very sensitive information to users that have been defined as administrators in the application. The information might be distributed by fax, email, IM, etc. based on user-defined settings. This method needs to retrieve a list of administrators. Historically, I would encapsulate building the set of administrators in the method with a call to an AdminSet class, or a call to a UserSet class that asks for a set of user objects that are administrators, or even via direct call(s) to the database. Then, I can call the method Notification.NotifyAdmins without fear of accidentally sending sensitive information to non-administrators.

I believe dependency injection calls for me to take an admin list as a parameter (in one form or another). This does facilitate testing, however, what's to prevent me from making a foolish mistake in calling code and passing in a set of NonAdmins? If I don't inject the set, I can only accidentally email the wrong people with mistakes in one or two fixed places. If I do inject the set aren't I exposed to making this mistake everywhere I call the method and inject the set of administrators? Am I doing something wrong? Are there facilities in the IoC frameworks that allow you to specify these kinds of constraints but still use dependency injection?

Thanks.

Answer 1

No, dependency injection does not require you to pass the admin list as a parameter. I think you are slightly misunderstanding it. However, in your example, it would involve you injecting the AdminSet instance that your Notification class uses to build its admin list. This would then enable you to mock out this object to test the Notification class in isolation.

Dependencies are generally injected at the time a class is instantiated, using one of these methods: constructor injection (passing dependent class instances in the class's constructor), property injecion (setting the dependent class instances as properties) or something else (e.g. making all injectable objects implement a particular interface that allows the IOC container to call a single method that injects its dependencies. They are not generally injected into each method call as you suggest.

Answer 2

You need to reverse your thinking.

If you have a service/class that is supposed to mail out private information to admins only, instead of passing a list of admins to this service, instead you pass another service from which the class can retrieve the list of admins.

Yes, you still have the possibility of making a mistake, but this code:

AdminProvider provider = new AdminProvider();
Notification notify = new Notification(provider);
notify.Execute();

is harder to get wrong than this:

String[] admins = new String[] { "[email protected]" };
Notification notify = new Notification(admins);
notify.Execute();

In the first case, the methods and classes involved would clearly be named in such a way that it would be easy to spot a mistake.

Internally in your Execute method, the code might look like this:

List<String> admins = _AdminProvider.GetAdmins();
...

If, for some reason, the code looks like this:

List<String> admins = _AdminProvider.GetAllUserEmails();

then you have a problem, but that should be easy to spot.

Answer 3

Other good answers have already been given, but I'd like to add this:

You can be both open for extensibility (following the Open/Closed Principle) and still protect sensitive assets. One good way is by using the Specification pattern.

In this case, you could pass in a completely arbitrary list of users, but then filter those users by an AdminSpecification so that only Administrators recieve the notification.

Perhaps your Notification class would have an API similar to this:

public class Notification
{
    private readonly string message;

    public Notification(string message)
    {
        this.message = message;

        this.AdminSpecification = new AdminSpecification();
    }

    public ISpecification AdminSpecification { get; set; }

    public void SendTo(IEnumerable users)
    {
        foreach(var u in users.Where(this.AdminSpecification.IsSatisfiedBy))
        {
            this.Notify(u);
        }
    }

    // more members
}

You can still override the filtering behavior for testing-purposes by assigning a differet Specification, but the default value is secure, so you would be less likely to make mistakes with this API.

For even better protection, you could wrap this whole implementation behind a Facade interface.