Constructor injection overuse

Question

I am looking for best practices of avoiding constructor injection overuse. For example I have Meeting entity which has few sub entities like shown below:

• Meeting
  1. MeetingContacts
  2. MeetingAttendees
  3. MeetingType
  4. Address
  5. MeetingCompanies
  6. MeetingNotes

MeetingService class looks like below:

public class MeetingService
{
    private readonly IMeetingContactRepository _meetingContactRepository;
    private readonly IMeetingAttendeeRepository _meetingAttendeeRepository;
    private readonly IMeetingTypeRepository _meetingTypeRepository;
    private readonly IAddressRepository _addressRepository;
    private readonly IMeetingCompanyRepository _meetingCompanyRepository;
    private readonly IMeetingNoteRepository _meetingNoteRepository;
    private readonly IMeetingRepositoy _meetingReposity;

    public MeetingService(IMeetingRepositoy meetingReposity, IMeetingContactRepository meetingContactRepository, IMeetingAttendeeRepository meetingAttendeeRepository, 
        IMeetingTypeRepository meetingTypeRepository, IAddressRepository addressRepository, 
        IMeetingCompanyRepository meetingCompanyRepository, IMeetingNoteRepository meetingNoteRepository)
    {
        _meetingReposity = meetingReposity;
        _meetingContactRepository = meetingContactRepository;
        _meetingAttendeeRepository = meetingAttendeeRepository;
        _meetingTypeRepository = meetingTypeRepository;
        _addressRepository = addressRepository;
        _meetingCompanyRepository = meetingCompanyRepository;
        _meetingNoteRepository = meetingNoteRepository;
    }

    public void SaveMeeting(Meeting meeting)
    {
        meetingReposity.Save();
        if(Condition1())
            _meetingContactRepository.Save();
        if(Condition2())
            _meetingAttendeeRepository.Save();
        if(Condition3())
            _meetingTypeRepository.Save();
        if(Condition4())
            _addressRepository.Save();
        if(Condition5())
            _meetingCompanyRepository.Save();
        if(Condition6())
            _meetingNoteRepository.Save();
    }
    //... other methods
}

Here are just seven dependencies but real code contains much more of them. I used different techniques described in the "Dependency Injection Constructor Madness" but I have not found how to deal with repository dependencies.

Is there any way how I can reduce the number of dependencies and keep the code testable?

Answer 1

Constructor overuse is just a symptom - it seems you are approximating a unit of work by having a "master" class that knows about the various elements of message persistence and plugs them into the overall save.

The downside is that each repository communicates its independence of the others by exposing a dedicated Save method; this is incorrect, though, as SaveMeeting explicitly states that the repositories are not independent.

I suggest identifying or creating a type that the repositories consume; this centralizes your changes and allows you to save them from a single place. Examples include DataContext (LINQ to SQL), ISession (NHibernate), and ObjectContext (Entity Framework).

You can find more information on how the repositories might work in a previous answer of mine:

Advantage of creating a generic repository vs. specific repository for each object?

Once you have the repositories, you would identify the context in which they would act. This generally maps to a single web request: create an instance of the common unit of work at the beginning of the request and hand it to all the repositories. At the end of the request, save the changes in the unit of work, leaving the repositories free to worry about what data to access.

This neatly captures and saves everything as a single unit. This is very similar to the working copy of your source control system: you pull the current state of the system into a local context, work with it, and save the changes when you're done. You don't save each file independently - you save them all at the same time as a discrete revision.

Answer 2

To expand a little bit on my comment above:

Since this question is directed towards how to manage repository dependencies, I have to assume that the MeetingService is managing some sort of persistent commit. In the past, when I have seen classes like MeetingService with that many dependencies, it is clear they are doing too much. So, you have to ask yourself, "what is my transaction boundary". In other words, what is the smallest commit that you can make that means that a meeting has been successfully saved.

If the answer is that a meeting is successfully saved after a call to meetingReposity.Save(); then that is all that MeetingService should be managing (for the commit).

Everything else is, essentially, a side effect of the fact that a meeting has been saved (notice that now we are speaking in the past tense). At this point, event subscription for each of the other repositories makes more sense.

This also has the nice effect of separating the logic in all of the conditions into subscriber classes that follow SRP to handle that logic. This becomes important when the logic of when the contact repository commits goes through a change, for example.

Hope this helps.

Answer 3

Each of the first three answers give important suggestions and ideas for dealing with the problem in the abstract. However, I may be reading too much into your example above, but this looks like a problem of too many aggregate roots, not too many dependencies per se. This has to do with either a lack in the persistence mechanism underlying your repository injection infrastructure, or a misconfiguration of the same.

Simply, the Contacts, Attendees, Notes, &c. should be composite properties of the Meeting itself (if only as links to separately managed Contact, &c. objects/data); therefore, your persistence mechanism should be saving them automatically.

Heeding Bryan Watts' adage that "constructor overuse is just a symptom," a couple of other possibilities:

• Your persistence mechanism should be handling persistence of the Meeting graph automatically, and is either misconfigured or lacks the capability to do this (all three that Bryan suggests do this, and I would add DbContext (EF 4.1+)). In this case, there should really only be one dependency--IMeetingRepositoy--and it can handle atomic saves of the meeting and its composites itself.
• SaveMeeting() is saving not just links to other objects (Contacts, Attendees, &c.) but is also saving those objects as well, in which case I would have to agree with dtryon that MeetingService and SaveMeeting() are doing far more than the names imply, and his mechanism could alleviate it.

Answer 4

Do you really need the repository functionality to be split into that many interfaces? Do you need to mock them separately? If not, you could have fewer interfaces, with more methods.

But let's assume your class really needs that many dependencies. In that case you could:

• Create a configuration object (MeetingServiceBindings) that provides all the dependencies. You could have a single configuration object per whole module, not just single service. I don't think there's anything wrong with this solution.
• Use a Dependency injection tool, like NInject. It's quite simple, you can configure your dependencies in code in one place and don't need any crazy XML files.