views:

94

answers:

3

Our project is using constructor-based Dependency Injection (we're using Unity as our container) and we have a number of constructors which have acquired a large number of parameters.

For example:

    public FooRequestService(
        ITransactionDataBuilder transactionDataBuilder,
        IMapper<SaveFooRequest, FooRequestMessage> saveFooRequestToFooRequestMapper,
        IFooRequestedThingRepository fooRequestedThingRepository,
        IFooRequestedThingBuilder fooRequestedThingBuilder,
        IMapper<IEnumerable<FooRequestedThing>, IEnumerable<FoundFooRequestedThing>> fooRequestedThingsToFoundFooRequestedThingsMapper,
        IAutomatedFooWcfService automatedFooService,
        IFooArrivalMessageProvider fooArrivalMessageProvider,
        ISeatCancellation cancellation,
        IPostSalesService postSalesService,
        ITransactionEnquiryService transactionService,
        IMapper<IEnumerable<FooRequestedThing>, List<ThingArrivedForFoo>> thingArrivalRequestDocumentMapper,
        IMapper<SaveFooRejected, FooRejectedMessage> saveFooRejectedToFooRejectedMapper,
        IPartialFooConfiguration partialFooConfiguration,
        ICurrentDateTimeProvider currentDateTimeProvider,
        IMapper<FooRequestMessage, List<FooRequested>> tracsFooRequestMapper,
        IThreeWayMerger<Transaction, TransactionService.Contract.Transaction, NotifyFooRequestedRequest, SaveFooRequest> saveFooRequestMerger,
        KioskWebServiceSoap kioskServiceGateway)
    {
        // code here to set injected values to member variables
    }

This class has a total of around 370 lines of code and represents a core part of the system, hence the large number of dependencies.

  • From a DI perspective, is is usual to have so many parameters passed to a constructor?
  • The general approach concerns me from an encapsulation and scoping aspect. All dependencies can be used from anywhere within a class, even if they should (strictly speaking) only be used from, for example, a single method
+3  A: 

Yes, this is the right approach. If you have so many dependencies in your class, maybe you should review your system architecture/design (maybe you can split the class in more smaller classes; it's just a suggestion, your class may be fine already), but from the DI point of view, it is ok.

Konamiman
A: 

I have experienced a similar behaviour when using Dependency injection.

I think you would benefit from using a parameter object refactoring.

Kindness,

Dan

Daniel Elliott
I had been considering this. For example, we have a number of services that could be grouped by an object, as well as mappers which map between service and domain objects.
Richard Ev
I think introducing a parameter object would just be covering up the core problem: FooRequestService simply has too many responsibilities. Break it up so that it only has one reason to change. Check out: http://en.wikipedia.org/wiki/Single_responsibility_principle
rcampbell
I agree with rrc7cz, parameter objects only complicate things here and don't really solve anything...
Mauricio Scheffer
+1  A: 

No, it shouldn't look like that. Your FooRequestService has way too many responsibilities if it has all those dependencies. You should refactor dependencies that are logically related into separate services, thus creating more reusable components while (more importantly) greatly reducing the dependencies your FooRequestService has.

With regard to your scoping comments, that's another symptom of a class with too many responsibilities. Those methods you want to scope your dependencies to should be extracted into a separate class.

James Gregory